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

Report percentual download progress in repository rules #18450

Closed
wants to merge 2 commits into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented May 19, 2023

If the HTTP response to a download request contains a Content-Length header, download progress is now reported as 10.1 MiB (20.2%) instead of 10.1 MiB (10,590,000B).

If the HTTP response to a download request contains a `Content-Length`
header, download progress is now reported as `10.1 MiB (20.2%)` instead
of `10.1 MiB (10,590,000B)`.
@fmeum fmeum marked this pull request as ready for review May 19, 2023 09:42
@fmeum fmeum requested a review from coeuvre May 19, 2023 09:42
@github-actions github-actions bot added awaiting-review PR is awaiting review from an assigned reviewer team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. labels May 19, 2023
@fmeum fmeum requested a review from meteorcloudy May 19, 2023 10:03
Copy link
Member

@meteorcloudy meteorcloudy left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

@meteorcloudy meteorcloudy added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels May 19, 2023
@fmeum
Copy link
Collaborator Author

fmeum commented May 19, 2023

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label May 19, 2023
@fmeum
Copy link
Collaborator Author

fmeum commented May 19, 2023

@coeuvre Not something for this PR, but I wonder whether we should consolidate the different ways download progress is presented across repo and remote cache downloads at some point. I don't know what the best format would be, but we could track and discuss in an issue.

@meteorcloudy
Copy link
Member

@bazel-io fork 6.3.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label May 19, 2023
@fmeum
Copy link
Collaborator Author

fmeum commented May 19, 2023

@meteorcloudy I pushed a small commit that handles edge cases I missed earlier.

@fmeum fmeum requested a review from meteorcloudy May 19, 2023 19:32
@coeuvre
Copy link
Member

coeuvre commented May 22, 2023

@coeuvre Not something for this PR, but I wonder whether we should consolidate the different ways download progress is presented across repo and remote cache downloads at some point. I don't know what the best format would be, but we could track and discuss in an issue.

Yes, I agree. Would you mind creating an issue for this?

@Pavank1992 Pavank1992 removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label May 22, 2023
@fmeum fmeum deleted the download-total-size branch May 22, 2023 15:56
iancha1992 pushed a commit to iancha1992/bazel that referenced this pull request May 22, 2023
If the HTTP response to a download request contains a `Content-Length` header, download progress is now reported as `10.1 MiB (20.2%)` instead of `10.1 MiB (10,590,000B)`.

Closes bazelbuild#18450.

PiperOrigin-RevId: 534035444
Change-Id: I1c5144555eda1890652b4d3f62b414292ba909d5
iancha1992 added a commit that referenced this pull request May 23, 2023
If the HTTP response to a download request contains a `Content-Length` header, download progress is now reported as `10.1 MiB (20.2%)` instead of `10.1 MiB (10,590,000B)`.

Closes #18450.

PiperOrigin-RevId: 534035444
Change-Id: I1c5144555eda1890652b4d3f62b414292ba909d5

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
fweikert pushed a commit to fweikert/bazel that referenced this pull request May 25, 2023
If the HTTP response to a download request contains a `Content-Length` header, download progress is now reported as `10.1 MiB (20.2%)` instead of `10.1 MiB (10,590,000B)`.

Closes bazelbuild#18450.

PiperOrigin-RevId: 534035444
Change-Id: I1c5144555eda1890652b4d3f62b414292ba909d5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants