-
Notifications
You must be signed in to change notification settings - Fork 31
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 support for Falcon Resource suffixes #19
Add support for Falcon Resource suffixes #19
Conversation
Codecov Report
@@ Coverage Diff @@
## master #19 +/- ##
==========================================
+ Coverage 97.36% 97.43% +0.06%
==========================================
Files 3 3
Lines 38 39 +1
==========================================
+ Hits 37 38 +1
Misses 1 1
Continue to review full report at Codecov.
|
@alysivji Any update on this? |
I have 4 hours booked tomorrow to dive into this. It's been a while since I've touched Falcon and I know folks are using this in prod -- want to make sure we don't break anything (especially now there is a v2 of Falcon) Just taking a quick look at the PR, it does look good. Do you mind updating the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
methods = resource_uri_mapping[resource]["methods"] | ||
|
||
for method_name, method_handler in methods.items(): | ||
docstring_yaml = yaml_utils.load_yaml_from_docstring(method_handler.__doc__) | ||
operations[method_name] = docstring_yaml or dict() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 this is a lot nicer
Closes #18.
Added support for the Falcon suffixes, using the Flacon's
router
.There is no need to specify the
suffix
manually. The methods for the correspondingResource
, as is done for thepath
, is fetched automatically inpath_helper
.Added tests for 100% coverage.