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

Add tests for progress report on size-less http downloads #369

Merged
merged 4 commits into from
Oct 27, 2023

Conversation

christian-monch
Copy link
Contributor

This PR adds a test to verify that http-download reports progress if no content-length is provided in the http-result headers. In addition the comments w.r.t. to download size calculation are updated to reflect the code.

This is a small add-on to #365 (which was already merged)

@christian-monch christian-monch requested a review from mih as a code owner May 23, 2023 13:29
@christian-monch christian-monch marked this pull request as draft May 23, 2023 13:31
@codecov
Copy link

codecov bot commented May 23, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (cc40c93) 91.96% compared to head (2943ccb) 92.17%.
Report is 23 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #369      +/-   ##
==========================================
+ Coverage   91.96%   92.17%   +0.20%     
==========================================
  Files         122      122              
  Lines        9101     9107       +6     
==========================================
+ Hits         8370     8394      +24     
+ Misses        731      713      -18     
Files Coverage Δ
datalad_next/url_operations/http.py 95.74% <ø> (ø)
datalad_next/url_operations/tests/test_http.py 60.86% <100.00%> (+3.15%) ⬆️

... and 8 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@christian-monch christian-monch force-pushed the enh-tests-chunked-progress-report branch 2 times, most recently from a7245f9 to c496b84 Compare June 6, 2023 08:32
@mih
Copy link
Member

mih commented Jul 27, 2023

This has been in draft mode for a long time. I know see that a review was requested (missed that due to the draft mode, sorry). I am closing this PR for now as part of a cleanup. Feel free to reopen when the conflicts are removed. Sorry for the hassle.

@mih mih closed this Jul 27, 2023
This commit updates the comments in url_oerations/http.py
to explain why and how we calculate downloaded bytes in the
absence of a content-length header.
This commit adds a test that verifies that
http-download progress is reported, if
no content-length header is provided.
@christian-monch
Copy link
Contributor Author

christian-monch commented Aug 25, 2023

Thanks, conflicts removed and reopened

@christian-monch christian-monch force-pushed the enh-tests-chunked-progress-report branch from c496b84 to 87040e1 Compare August 25, 2023 09:03
@christian-monch christian-monch marked this pull request as ready for review August 25, 2023 09:48
@mih mih added this to the 1.0.3 milestone Oct 27, 2023
@mih mih added the semver-patch PR implies patch-level version increment (API stable) label Oct 27, 2023
Copy link
Member

@mih mih left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thx! Before merging, this needs a changelog, because it is not just adding a test.

@mih
Copy link
Member

mih commented Oct 27, 2023

Sorry, no, only comments are adjusted.

@mih mih merged commit 7f030a4 into datalad:main Oct 27, 2023
8 checks passed
@mih mih modified the milestones: 1.0.3, 1.1 Dec 6, 2023
@christian-monch christian-monch deleted the enh-tests-chunked-progress-report branch July 16, 2024 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-patch PR implies patch-level version increment (API stable)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants