-
-
Notifications
You must be signed in to change notification settings - Fork 857
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
Add progress to streaming download #1268
Add progress to streaming download #1268
Conversation
Co-authored-by: Florimond Manca <florimond.manca@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still need to ponder on this a bit, but very neat implementation (code, docs and tests)!
Don't think so, since that's directly available as |
Co-authored-by: Florimond Manca <florimond.manca@gmail.com>
Great, thanks! I think this has helped me. understand why I'm a bit wary of It seems to me that it's more likely to lead to brittle code. Also it's not super clear what the behaviour is expected to be in different cases. Eg...
In contrast if we used response = httpx.get(...)
response.num_bytes_downloaded # Determine the final download size. It is true that num_bytes_downloaded = 0
with httpx.stream("...") as response:
total = ...
with tqdm(total=...) as progress:
for chunk in response.iter_bytes():
download_file.write(chunk)
progress.update(response.num_bytes_downloaded - num_bytes_downloaded)
num_bytes_downloaded = response.num_bytes_downloaded What do we think? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lovely bit of functionality to work towards yup!
Really liking the docs example here.
Possible that we ought to be going for a total downloaded bytes API rather than a chunk size API?
I changed For now we can state that there is a correlation between the amount of data (lets say |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good with num_bytes_downloaded
too, yup 👍
Co-authored-by: Florimond Manca <florimond.manca@gmail.com>
Co-authored-by: Florimond Manca <florimond.manca@gmail.com>
Have made a couple of minor docs tweaks. |
Having found #1208, I tried to track last downloaded raw chunk of the response.
The question is: