Convert API response JSON to Python dict #26
Merged
Conversation
This file contains 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
|
This PR addresses the issue #25 |
rhymiz
requested changes
May 19, 2021
There was a problem hiding this comment.
@lucioseki my proposal here is to change response.text to response.json() in the graphql client. That's the source of the issue.
Note: requests will raise an exception if you call .json on a response object that does not contain a 2xx or 4xx status code, that can either be handle explicitly by you, or you can call response.raise_for_status() before moving on to calling response.json.
rhymiz
reviewed
May 19, 2021
Some methods in this library are returning the API response JSON string without converting it to a Python dict. This is forcing the caller to selectively call json.loads depending on the retrieved resource (e.g. `users` resource requires json.loads, but `items` doesn't). This patch makes all the API responses to be converted to a Python dict, so the caller can consistently use the return value as a dict regardless the requested resource.
Currently, the library converts the response.text returned by the GraphQL client to a Python dict using json.loads. But the requests library already has a builtin JSON decoder that converts a JSON response to a Python dict. This patch makes the GraphQL return the response.json() result. The callers no longer need to convert the return value to a dict.
also some code cleanup
lucioseki
force-pushed
the
convert_response_json_to_dict
branch
from
5eae55b to
669bb0e
Compare
rhymiz
approved these changes
May 19, 2021
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.
Some methods in this library are returning the API response JSON
string without converting it to a Python dict.
This is forcing the caller to selectively call
json.loadsdepending on the retrieved resource (e.g.
usersresource requiresjson.loads, butitemsdoesn't).This patch makes all the API responses to be converted to a Python
dict, so the caller can consistently use the return value as a dict
regardless the requested resource.