micropython-lib
micropython-lib copied to clipboard
requests: Make possible to override all request headers and allow raw data upload
This removes all the hard-coded request headers from the requests module so they can be overridden by user provided headers dict. Furthermore allow streaming request data without chunk encoding in those cases where content length is known but it's not desirable to load the whole content into memory. Also some servers (e.g. nginx) reject HTTP/1.0 requests with the Transfer-Encoding header set. The change should be backwards compatible as long as the user hasn't provided any of the previously hard-coded headers.
Thanks for the contribution, this change looks very reasonable.
The only concern is that it may use a little more memory, and make more allocations when it populates the headers dict. The way it works prior to this patch is that the write of the hard-coded header is basically allocation free.
I had the same though but choose to copy the auth
behavior that was already there: https://github.com/micropython/micropython-lib/blob/80b21be6f4acadf6132b22129b90309991c93160/python-ecosys/requests/requests/init.py#L59
But I can change it so it writes to the socket directly and doesn't populate the headers dict. Would that make sense?
I can change it so it writes to the socket directly and doesn't populate the headers dict.
How exactly would you do that, do you mean something like the current if "Host" not in headers: s.write(...)
?
If you could do that without making it too complex, then that might be preferable. Maybe keep both versions so they can be compared.
This commit writes headers directly to the socket: https://github.com/kapetan/micropython-lib/commit/b3f795abc32e8b3a044b9e3714c717defb347443
I think that could work.
I also added a test file where I mock the usocket
module. The test file is not strictly necessary but I think it's nice to have. It's not possible to run the requests code with CPython so I couldn't use unittest
. CPython doesn't allow mixing bytes and strings when formatting.
As a side effect, this should fix #839.
Thanks @kapetan for following up with that alternative version.
But I think the version you have in this PR (creating a dict of headers to write out) is cleaner and easier to maintain (especially since we'll want to update to use HTTP/1.1).
Can you please add the test that you wrote for that alternative version to this PR? Then it should be good to merge.
I closed this pull-request by accident so I created a new one with the original changes and the test file: https://github.com/micropython/micropython-lib/pull/881