Skip to content

fix: re-introduce pagination#3521

Merged
rchincha merged 2 commits into
project-zot:mainfrom
ljakimczuk:fix-pagination-on-sync
Nov 7, 2025
Merged

fix: re-introduce pagination#3521
rchincha merged 2 commits into
project-zot:mainfrom
ljakimczuk:fix-pagination-on-sync

Conversation

@ljakimczuk

Copy link
Copy Markdown
Contributor

What type of PR is this?

A bug / regression.

Which issue does this PR fix:

#2715.

What does this PR do / Why do we need it:

Without it, Zot only gets first 100 repositories from the registry. This PR makes it continue building the complete list.

If an issue # is not available please add repro steps and logs showing the issue:

N/A

Testing done on this change:

So far I tested this in my environment, no unit tests added yet.

Automation added to e2e:

N/A

Will this break upgrades or downgrades?

It should not.

Does this PR introduce any user-facing change?:

N/A

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Signed-off-by: Łukasz Jakimczuk <ljakimczuk@gmail.com>
@ljakimczuk
ljakimczuk force-pushed the fix-pagination-on-sync branch from a55cc6c to cda417f Compare November 7, 2025 12:21
@codecov

codecov Bot commented Nov 7, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.47619% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.71%. Comparing base (911244b) to head (381adee).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/extensions/sync/remote.go 90.47% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3521      +/-   ##
==========================================
- Coverage   91.73%   91.71%   -0.02%     
==========================================
  Files         185      185              
  Lines       25656    25674      +18     
==========================================
+ Hits        23535    23547      +12     
- Misses       1370     1374       +4     
- Partials      751      753       +2     

☔ 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.

andaaron
andaaron previously approved these changes Nov 7, 2025
@andaaron

andaaron commented Nov 7, 2025

Copy link
Copy Markdown
Contributor

The only strange part is the coverage decrease in
image
Maybe before this change there was some kind of error raised in case the limit was reached, and right now it isn't anymore?

Signed-off-by: Łukasz Jakimczuk <ljakimczuk@gmail.com>
@ljakimczuk

ljakimczuk commented Nov 7, 2025

Copy link
Copy Markdown
Contributor Author

@andaaron now when I look at it, maybe it is my bad, I changed assignment to short declaration, I guess that makes err a new local variable inside the loop then the error return from getRepoList is lost, changed that back to assignment.

@ljakimczuk

Copy link
Copy Markdown
Contributor Author

@andaaron all looks good now.

@ljakimczuk
ljakimczuk requested a review from andaaron November 7, 2025 15:02

@andaaron andaaron left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for fixing this.

@rchincha
rchincha merged commit aaba362 into project-zot:main Nov 7, 2025
41 checks passed
@ljakimczuk

Copy link
Copy Markdown
Contributor Author

@rchincha @andaaron I saw after merging to the main branch, the codecov jobs failed there again, is it something normal?

@andaaron

andaaron commented Nov 7, 2025

Copy link
Copy Markdown
Contributor

The sync tests are ok from the point of view of functional testing, but are more async than sync, so not as good as we would want them to be for consistent test coverage on all code paths. They may or may not hit some code lines depending on timing (mostly on reaching some errors).

I have added additional tests in the past couple of months to consistently cover some of these error branches, but it looks like there is still work to do.
Those 2 lines were covered when we merged your PR, but, as far as I remember, they were not before your last fix. One of them is in your new function, the other is not.

@ljakimczuk

Copy link
Copy Markdown
Contributor Author

Thanks for explanetion!

@ljakimczuk

Copy link
Copy Markdown
Contributor Author

@rchincha sorry for bothering you, but can you give me raw estimates on when we may expect the next Zot release?

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.

3 participants