-
Notifications
You must be signed in to change notification settings - Fork 99
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: add type for failure during deleting dangling referrer index #482
Conversation
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #482 +/- ##
==========================================
+ Coverage 72.84% 72.92% +0.08%
==========================================
Files 49 49
Lines 4518 4528 +10
==========================================
+ Hits 3291 3302 +11
Misses 919 919
+ Partials 308 307 -1
|
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Is the digest of the manifest that failed to be deleted included in the errro? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Although please answer Sajay's question. Digest appears to be included to me.
I'm struggling to understand why we'd delete content for the user: #479 (comment) |
It implicitly implied in the |
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we may have more specific errors in the future. It would be better if we can introduce a generic error to avoid error type blow up.
A sample error can be
type ReferrersError struct {
Op string
Subject ocispec.Descriptor
Err error
}
Other examples can be found by
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Updated, made it more generic to a |
Ignorable is confusing to me. It’s up to the caller to decide that IMHO. |
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qweeah It would be better if we can have a runnable example to show how this error can be parsed. Basically, we can have a mock registry which does not support delete.
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Resolves #479