-
Notifications
You must be signed in to change notification settings - Fork 63
Examine "next" branch #44
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
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
gene1wood
commented
Apr 13, 2018
- Take a look at this branch
- Figure out what it does
- Decide if we want to use this
- Merge it or make an issue capturing the idea or close and delete the branch
This is, of course, very subjective, but the rule currently is: * Most things are camelCase * Constants are capatalized and under_scored * Function/Method named parameters are under_scored * Members of the ConnectionProperies class are under_scored * The Content "media-type handlers" still use under_scores This is a technical limitation, and isn't likely to change. It does (helpfully) emphasize them as "special."
Now, we spell it the same way as the official GitHub branding. - test.py has also been updated to match
In the Client class: setConnectionProperties() - Call caseConvertHeaders() instead of duplicating its code caseConvertHeaders() - Only do case conversion New function updateWithDefaultHeaders() - Call caseConvertHeaders() on the input - Do the defaultHeaders update which we removed from caseConvertHeaders() request() - Call updateWithDefaultHeaders() instead of caseConvertHeaders()
Since we plant to support request-body's in a parallel way, it only makes sense to name the two classes parallelly. On the outside, this shifts focus from the content-type to the response body the basic concept. - Rename Content -> ResponseBody, et al - Rename Content.processBody -> ResponseBody.proces - Rename Client.request()//content -> resBody
…t-stream As per RFC-2068, Section 7.2.1, line 2364, we use the official 'application/octet-stream' media-type for unknown entity-bodies, instead of the ad-hoc 'x-application/unknown' media-type.
We've been ignoring the standard (RFC 2068) for a while now, instead expecting every media-type to include a charset parameter (and only that). Now, we corectly handle multiple, varied media-type parameters, giving our media-type handlers that much more flexibility. getContentType removed, superseded by parseContentType new method parseContentType - Stores self.mediatype - Stores self.ctypeParameters, a dict of the parameters - Sets charset correctly, even if it was implicit, so the handler's don't have to implement any "default logic" for it
Body contains those parts of a response body which are conceptually or algorithmically shared with a request body, such as content-type decoding. This is in preparation for a new RequestBody class.
Client: - The HTTP methods which accept a body now pass it unchanged to Client.request - Client.request now constructs a RequestBody, and uses it to process the user-supplied body RequestBody Handles charset encoding and media-type serialization; it is structured very similarly to ResponseBody
Before the RequestBody was introduced, all request bodies were assumed to be JSON data. We'd like to keep this behavior because * JSON is expected to be a common value for this field * It's one less thing existing users will need to upgrade - Add 'content-type' to the DEFAULT_HEADERS request - Delete the content-type header in case there is no body This allows non-body methods (like get, head) to work unchanged. (Sending a content-type header when no body is expected might theoretically break some servers.)
- TODO: Reuse TCP connections - Update "Parse Content-Type" bullet
We use xml.dom.minidom to de-serialize the results.
…eaders In case the API does not supply extra_headers, self.request() will fail trying to delete the 'content-type' header from them. Including the DEFAULT_HEADERS (always) ensures that 'content-type' is defined
Closed
The good stuff from this PR is now in #45. Once we merge that or decide we don't want the content, we can close this PR, not merge it and delete the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.