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

feat: track progress #852

Closed
wants to merge 15 commits into from
Closed

feat: track progress #852

wants to merge 15 commits into from

Conversation

shizhMSFT
Copy link
Contributor

@shizhMSFT shizhMSFT commented Dec 2, 2024

Important

This PR resolves #839 and is experimental.
This PR is also a duplicate of the internal/progress package in oras-project/oras#1524.

This PR introduce a new package progress to track the progress of reading content.

@shizhMSFT
Copy link
Contributor Author

Discussions should go to #839

Copy link

codecov bot commented Dec 2, 2024

Codecov Report

Attention: Patch coverage is 86.84211% with 10 lines in your changes missing coverage. Please review.

Project coverage is 75.78%. Comparing base (e4dc64e) to head (fc11b2f).
Report is 17 commits behind head on main.

Files with missing lines Patch % Lines
progress/manager.go 0.00% 6 Missing ⚠️
progress/tracker.go 94.28% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #852      +/-   ##
==========================================
+ Coverage   75.72%   75.78%   +0.05%     
==========================================
  Files          63       65       +2     
  Lines        6011     6087      +76     
==========================================
+ Hits         4552     4613      +61     
- Misses       1078     1090      +12     
- Partials      381      384       +3     

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

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Shiwei Zhang <[email protected]>
Signed-off-by: Shiwei Zhang <[email protected]>
Signed-off-by: Shiwei Zhang <[email protected]>
Signed-off-by: Shiwei Zhang <[email protected]>
Signed-off-by: Shiwei Zhang <[email protected]>
Signed-off-by: Shiwei Zhang <[email protected]>
Signed-off-by: Shiwei Zhang <[email protected]>
Signed-off-by: Shiwei Zhang <[email protected]>
Signed-off-by: Shiwei Zhang <[email protected]>
Signed-off-by: Shiwei Zhang <[email protected]>
@shizhMSFT shizhMSFT added the experimental Issues or pull requests depending on WIP specs label Dec 31, 2024
Copy link

This PR is stale because it has been open for 45 days with no activity. Remove the stale label or comment to prevent it from being closed in 30 days.

@github-actions github-actions bot added stale Inactive issues or pull requests and removed stale Inactive issues or pull requests labels Feb 14, 2025
@deitch
Copy link
Contributor

deitch commented Mar 19, 2025

@shizhMSFT any progress on this?

@shizhMSFT
Copy link
Contributor Author

@deitch I have good news and bad news for this track progress effort.

The good news is that we have this package integrated in the oras CLI v1.3.0-beta.2 (see code). You can check the code and test it out.

The bad news is that the code / design in this PR does not match the quality for generic use in oras-go. There are several issues found when we did the integration:

  1. The current progress model only covers the blob downloading operation and does not cover things like resolving the tags. Discussion is there in Copy progress - feature request #839 (comment)
  2. Degraded performance. If an oras-go user invokes the implementation of this PR directly, the performance will be roughly 10x slower due to too frequent progress event report and rendering. The oras CLI resolves this by introducing two strategies for different state reports. Precisely, transmitting events can be dropped if there is no CPU cycle for them (stateless like UDP) and other events will block until the render consumer picks it (stateful like TCP) so that critical events won't be missed (see code). However, which strategy should be used for an event (note: an event can be user defined) is purely application specific. Currently, I don't have a solution for making it generic.

Therefore, I'd like to abandon this PR and try to find a better design for the progress package in oras-go in the future.

Our overall strategy for having a new package in oras-go is that we first experiment in oras CLI and iterate it to be mature as well as validate the scenarios for real requirements. Once the new package is mature, we can promote it from the oras CLI to oras-go.

@shizhMSFT shizhMSFT closed this Mar 20, 2025
@shizhMSFT shizhMSFT deleted the progress branch March 20, 2025 06:13
@deitch
Copy link
Contributor

deitch commented Mar 20, 2025

Our overall strategy for having a new package in oras-go is that we first experiment in oras CLI and iterate it to be mature as well as validate the scenarios for real requirements. Once the new package is mature, we can promote it from the oras CLI to oras-go.

That makes sense, keeps the core critical library more robust.

The good news is that we have this package integrated in the oras CLI v1.3.0-beta.2 (see code). You can check the code and test it out.
The bad news is that the code / design in this PR does not match the quality for generic use in oras-go. There are several issues found when we did the integration

The bad news is it is not in core, the good news is that, the same way CLI got it, I could get the same effect by adding my own code like CLI?

I looked at the code, I could not quite figure out how I would leverage it. How would I hook it into an oras.Copy() to get the progress?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental Issues or pull requests depending on WIP specs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Copy progress - feature request
2 participants