Skip to content
This repository has been archived by the owner on Jan 11, 2021. It is now read-only.

Omit empty resources #318

Merged
merged 5 commits into from
Jul 17, 2015
Merged

Conversation

mverteuil
Copy link
Contributor

I've amended the code for handling resource access because I've found a bug that is exercised by one of my use cases. If all of the APIs provided by a Resource are excluded by handle_resource_access, and that resource is actually just a label to collect a bunch of similar APIs together, then that Resource will show up. This is problematic because:

  • It's generally messy looking
  • It's sort of confusing and looks broken that the top level resources aren't expanding when clicked
  • It may expose details of your business that you would otherwise prefer to keep private

The problem is solved simply by doing the check at an earlier stage of processing.

@mverteuil
Copy link
Contributor Author

Before

broken-api

After

fixed-api

for path in resources:
apis.append({'path': '/%s' % path, })

apis = [{'path': '/' + path} for path in self.get_resources()]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use '/%s' % path?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because David Beazley told me it's ok.

python_cookbook__3rd_edition_pdf__page_77_of_706_

But in any event, the string formatting in this file is pretty schizophrenic and '%s' % value is less favourable to '{}'.format(value) or '{0}'.format(value) says python docs.

@ariovistus
Copy link
Contributor

rebase, perhaps?

By filtering before trying to determine the top-level resources, empty top-level
resources will be correctly ignored.
@mverteuil
Copy link
Contributor Author

Rebased 💇

@mverteuil
Copy link
Contributor Author

Investigating the test failures now.

@mverteuil
Copy link
Contributor Author

Oh I rebased onto an old HEAD, should be easily resolved.

@mverteuil
Copy link
Contributor Author

I wasn't aware that filter doesn't return a list in python3, but I am now. ;)

It was a good thing your test exercised this, or it would have probably caused some curious issues.

ariovistus pushed a commit that referenced this pull request Jul 17, 2015
@ariovistus ariovistus merged commit 100cbc6 into marcgibbons:master Jul 17, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants