Requests support #32
Requests support #32
Conversation
Codecov Report
@@ Coverage Diff @@
## master #32 +/- ##
==========================================
- Coverage 98.47% 98.32% -0.16%
==========================================
Files 18 18
Lines 919 1015 +96
==========================================
+ Hits 905 998 +93
- Misses 14 17 +3
Continue to review full report at Codecov.
|
| @@ -107,6 +110,73 @@ def mimetype(self): | |||
| return self.request.mimetype | |||
|
|
|||
|
|
|||
| class RequestsOpenAPIRequest(BaseOpenAPIRequest): | |||
|
|
|||
| def __init__(self, response, path_pattern, regex_pattern, server_pattern): | |||
There was a problem hiding this comment.
I think it should accept request object (not response).
http://docs.python-requests.org/en/master/api/#lower-level-classes
User don't need to have response to be able to validate request.
There was a problem hiding this comment.
I don't think the requests library explicitly uses a request object in its normal flow. Instead the response starts out as a function. You don't get an actual response object until after the http result is returned.
response = requests.post('http://httpbin.org/post', data = {'key':'value'}) request = response.request
There was a problem hiding this comment.
Ah, I misread what you were getting at. I pass in the response because of the way that the requests library distributes its data. For example, in order to get cookies it needs the response object even though cookies are a part of the request model in your interface.
I also use it to extract the correct mimetype in case the request doesn't have an 'Accept' header.
When a request doesn’t have a feature, like query params, then the requests library will omit it. Unfortunately this causes a property not found error in the previous code.
|
Thank you for the contribution. Closing in favor of #209 |
This is a preliminary version I'm submitting for review, so I am expecting to make some changes. If I sound critical it is meant constructively because I really think you did a great job with this library!
Dealing With the Lack of A Path Pattern
The Requests library does not have a concept of path pattern. This means that one must be manually entered when passing the request into the wrapper or choosing another way. I chose another way for the following reasons.
Solution
We already have all the possible path and server patterns that should exist in our API defined in the spec. I took these and converted them to regex expressions. This allowed the new logic to identify what should be the correct path using only the request url. Because I didn't want to have to rebuild the regex every time I made a new request wrapper I created a RequestsFactory.
Proposed Changes
I think that the functionality of the RequestsFactory should be moved into the core library. That way the regex could be created when the spec is thereby eliminating the need for the RequestsFactory. I see this being used as follows.
The same process could be used for path parameters and server host urls. This should greatly simplify the implementation of new wrappers.
Please let me know what you think. The current implementation should be completely functional in its current form so feel free to try it out. If you think there is enough value to merge it now then let me know and I'll update the documentation. I'd also be more than happy to refactor it after you merge #25 .