fix: address code review comments #3942
Conversation
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses follow-up review feedback from the multi-tag digest push work by improving error observability and making controller port selection safer and less racy in tests.
Changes:
- Add an error log when snapshotting prior tag state from MetaDB fails during multi-tag digest pushes.
- Make
Controller.chosenPortatomic and update reads/writes accordingly. - Reduce test flakiness by replacing fixed sleeps with
StartAndWait/ port-wait logic and modernizing a small constants test assertion.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
pkg/meta/hooks.go |
Adds explicit error logging when prior tag state snapshotting fails for multi-tag digest pushes. |
pkg/api/controller.go |
Stores the chosen port in atomic.Int32 to avoid races between Run() and concurrent GetPort() readers. |
pkg/api/controller_test.go |
Replaces some Sleep() calls with readiness waits; adds helper to wait for kernel-chosen port population; updates auto-port log assertion flow. |
pkg/api/constants/consts_test.go |
Switches to testify/assert for a clearer equality assertion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
See https://github.com/project-zot/zot/actions/runs/24045271222/job/70126983674?pr=3942 Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3942 +/- ##
=======================================
Coverage 91.53% 91.53%
=======================================
Files 195 195
Lines 27911 27922 +11
=======================================
+ Hits 25547 25558 +11
Misses 1530 1530
Partials 834 834 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…le poll loop Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
From #3885 (review)
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.