search

SpotlightKid / mrequests

An HTTP client library (not only) for MicroPython with an API similar to requests
MIT License
49 stars 10 forks source link

Large file chunk support #2

Closed kmorwood closed 1 year ago

kmorwood commented 2 years ago

When a server sets the header to include 'Transfer-encoding: chunked' it also OMITS the 'Content-length: ' from the header. That means default _content_length remains 0. Then the save() function always creates the file and then exits...since 'remain = self._content_size - read' will always result in a value of 0.

The response parser was correctly detecting the 'Transfer-encoding: chunked' and setting the necessary flags so I added detection for that condition and just read the stream into the file until the chunk read returns nothing from the stream.

I have tested the change extensively within my own project that is using mrequests but am not sure how to add such testing to this repository. The external server has the responsibility of setting the transfer encoding value. Please advise.

SpotlightKid commented 2 years ago

Sorry for taking waaayyyy to long to address this! :-(

If you are still interested, could you test your use cases with commit 6c7a943666adc51fdf70e60aaffdafc6ada2458f ?

(I didn't use your patch because from my analysis it would break chunked reading/saving of responses without chunked transfer encoding.)

kmorwood commented 2 years ago

Hey! Don't worry about it at all. I understand the rigours of life and finding time to do everything is tough.

I had actually tested both the chunked and un-chunked use cases but I will set up a test environment to see what your changes do to my code. :)

It might take me a few days/weeks to get to this as I am in the middle of a release cycle in my day job.

I'll read the diff now at least.

:)

SpotlightKid commented 2 years ago

I had actually tested both the chunked and un-chunked use cases

Well, not actually break the latter, but download it all at once, if my interpretation was correct.

kmorwood commented 2 years ago

So...I reviewed your version of the code. It seems you have done a more integrated change in the save method. I intentionally didn't change the 'content-length' part of the implementation because I didn't have all of the test paths for that part.

Ultimately the big change is the detection of the 'transfer-encoding: chunked' header which engages a different mode of byte transfer in the save function. Your code does the same thing that my pull request did...but does it better. :)

I'll test it as soon as I can but it looks good.

SpotlightKid commented 1 year ago

Due to no further feedback, I'm closing this PR, assuming that the functionality / fixes it wanted to add has been successfully implemented via 6c7a943666adc51fdf70e60aaffdafc6ada2458f.