Skip to content
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

connection.request auto json encode #41

Open
Bloodiko opened this issue Dec 30, 2023 · 2 comments
Open

connection.request auto json encode #41

Bloodiko opened this issue Dec 30, 2023 · 2 comments

Comments

@Bloodiko
Copy link

The auto json encode on the ["data"] field is not a good idea, as "data" is usually supposed to be the body.
If you have a JSON body, you should send the "json" parameter instead of data, as they are mutually exclusive.

Excerpt from aiohttp doc

json – Any json compatible python object (optional). json and data parameters could not be used at the same time.

you dont even need to parse the data field into json, as aiohttp will do it for you.

As some of the lcu-api functions require plain-text / form encoded data, the data field should be passed along and not parsed.

Suggestion

Remove 'data'-autoparsing on the request and adjust the pydoc to the following.

Send JSON body data via the **json** keyword argument. The **data** keyword can be used for form based data. 
Both keywords (json and data) are mutually exclusive. Make sure to only send one.
@sousa-andre
Copy link
Owner

Woah that's a smarter way to do it, thanks for pointing it out. I'll now add json as an alias for data, and later, on a major version release, I'll cast the lcu-driver data to aiohttp data without the encoding. Sounds good?

@Bloodiko
Copy link
Author

Yep sounds good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants