-
-
Notifications
You must be signed in to change notification settings - Fork 51
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
Swagger for search #630
Swagger for search #630
Conversation
Codecov Report
@@ Coverage Diff @@
## master #630 +/- ##
==========================================
- Coverage 93.52% 93.48% -0.05%
==========================================
Files 284 284
Lines 25116 25141 +25
==========================================
+ Hits 23490 23503 +13
- Misses 1626 1638 +12
Continue to review full report at Codecov.
|
42e2b9c
to
ea3de1a
Compare
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.
Looks really close--just a couple more tweaks.
guillotina/api/service.py
Outdated
data = self.request.url.query | ||
for parameter in self.__config__['parameters']: | ||
if parameter['in'] == 'query': | ||
if parameter['schema']['type'] == 'integer': |
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.
Could we handle KeyError here?
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.
keyError for parameter['schema']['type']?
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.
Yes, if they define a parameter without the schema
defined
I imagine it already default to string so you don't need to define this.
guillotina/api/service.py
Outdated
if parameter['in'] == 'query': | ||
if parameter['schema']['type'] == 'integer': | ||
try: | ||
int(data[parameter['name']]) |
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.
Before all this, shouldn't we first check if name
is in data
?
guillotina/api/service.py
Outdated
}) | ||
else: | ||
pass | ||
if parameter['required'] and parameter['name'] not in data: |
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.
what if they don't define required
?
Maybe the whole check should be wrapped in KeyError
handling?
35faae3
to
5f3654a
Compare
5f3654a
to
a3d6a1c
Compare
No description provided.