-
-
Notifications
You must be signed in to change notification settings - Fork 227
feat: Allow path parameters to be positional args [#429]. Thanks @tsotnikov! #464
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
Conversation
Codecov Report
@@ Coverage Diff @@
## main #464 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 47 47
Lines 1594 1614 +20
=========================================
+ Hits 1594 1614 +20
Continue to review full report at Codecov.
|
FYI @mtovts this will conflict with #458 once merged. I'll plan on waiting on yours first so I can handle the conflicts here. @forest-benchling this is based on another stale PR I've been neglecting, I could use a fresh set of eyes on it if you have a chance to take a look. |
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.
Thanks Dylan!
end_to_end_tests/golden-record/my_test_api_client/api/parameters/multiple_path_parameters.py
Show resolved
Hide resolved
OpenAPI Specification indicates that if parameter location is `path` the `required` property is REQUIRED and its value MUST be `true` Ref: https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#parameter-object With this in mind, this PR: - Tighten the parsing by throwing an error for path parameters with required=false - Update the templates to pass the path parameters to the function as positional arguments instead of kwargs to improve usability
Also fix typo Co-authored-by: Dylan Anthony <[email protected]>
c2adba8
to
aa63b86
Compare
ff3211a
to
f0fe788
Compare
This replaces #429 so I can rebase and resolve conflicts.