Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add Resource predicates support #344

Merged
merged 8 commits into from
Nov 25, 2016
Merged

Conversation

vmaksymiv
Copy link
Contributor

It allows to register different Resources with the same path.
Ex.

@resource(name='FlyingUnicorn', path='/unicorns/{lovely_name}', unicorn_aptitude='fly')
class FlyingUnicorn():
    @view
    def get(self):
         # FlyingUnicorn stuff

@resource(name='MagicUnicorn', path='/unicorns/{lovely_name}', unicorn_aptitude='do magic')
class MagicUnicorn():
    @view
    def get(self):
         # MagicUnicorn stuff

@leplatrem
Copy link
Contributor

Thanks for your contribution!

The feature does not look irrelevant to me. I witl let @almet confirm, but it is sure that it can't land in master without tests :)

@almet
Copy link
Contributor

almet commented Jan 20, 2016

I confirm, this would be useful, but needs testing.

@leplatrem
Copy link
Contributor

@vmaksymiv would you have some time to contribute a couple of tests ?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling db312da on openprocurement:master into 66f1d6a on Cornices:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling db312da on openprocurement:master into 66f1d6a on Cornices:master.

@coveralls
Copy link

coveralls commented Nov 25, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 15da363 on openprocurement:master into 66f1d6a on Cornices:master.

@vmaksymiv
Copy link
Contributor Author

Hello!
Sorry for the late response.
We added tests for "custom predicates" feature and they successfully passed.
But to make all test checks work we excluded from coverage one line (explained in comment to that line).
We couldn't find parameters set for request to reach this line. Maybe you can find better solution than excluding the line?

@leplatrem
Copy link
Contributor

Great! Thanks for your additional efforts on this!

Indeed, I agree with your comment. This particular line is impossible to reach, and didn't feel confident enough to get rid of it during the 2.0 refactor (See https://github.com/Cornices/cornice/blob/master/tests/test_pyramidhook.py#L280-L289).

@leplatrem leplatrem merged commit 3f6eb1d into Cornices:master Nov 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants