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

api: Disallow asset progress from going back #1688

Merged
merged 2 commits into from
Apr 13, 2023
Merged

Conversation

victorges
Copy link
Member

What does this pull request do? Explain your changes. (required)

This disallows the progress displayed on asset objects to go back in time.

This avoids the potential confusion on the higher level asset objects. Notice that
the real progress and retry count are still available on the lower-level task objects,
for power-users that do want to display that in any UI.

Specific updates (required)

  • Make sure to never make the asset progress smaller than before

How did you test each of these updates (required)
yarn test
staging check

Does this pull request close any open issues?
Fixes API-41

Checklist

  • I have read the CONTRIBUTING document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.

@victorges victorges requested a review from a team as a code owner April 11, 2023 23:09
@linear
Copy link

linear bot commented Apr 11, 2023

API-41 Prohibit asset progress to go back in %

When the processing status is updated for VOD, it jumps back and forth from 7% to 32% to 7%.

See the video from the user:

https://drive.google.com/file/d/1BzjJtdWAhfu87fYV5JtyoePhPtqS1sQN/view?usp=sharing

@vercel
Copy link

vercel bot commented Apr 11, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
livepeer-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 11, 2023 11:09pm

@codecov
Copy link

codecov bot commented Apr 11, 2023

Codecov Report

Merging #1688 (0f3c9ad) into master (c1b73a7) will decrease coverage by 0.04325%.
The diff coverage is 40.00000%.

Impacted file tree graph

@@                 Coverage Diff                 @@
##              master       #1688         +/-   ##
===================================================
- Coverage   52.98203%   52.93878%   -0.04325%     
===================================================
  Files             74          74                 
  Lines           4896        4900          +4     
  Branches         970         972          +2     
===================================================
  Hits            2594        2594                 
- Misses          1969        1972          +3     
- Partials         333         334          +1     
Impacted Files Coverage Δ
packages/api/src/controllers/task.ts 31.57895% <40.00000%> (+0.57120%) ⬆️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1b73a7...0f3c9ad. Read the comment docs.

Impacted Files Coverage Δ
packages/api/src/controllers/task.ts 31.57895% <40.00000%> (+0.57120%) ⬆️

... and 1 file with indirect coverage changes

Copy link
Member

@adamsoffer adamsoffer left a comment

Choose a reason for hiding this comment

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

LGTM 🚢

@victorges victorges merged commit 73c1082 into master Apr 13, 2023
11 checks passed
@victorges victorges deleted the vg/fix/asset-progress branch April 13, 2023 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants