-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
database: remove LayerWithContent from interface #619
Conversation
KeyboardNerd
commented
Sep 11, 2018
- Layer is renamed to LayerMetadata
- LayerWithContent is renamed to Layer
- Remove redundant interfaces to pertain layer's metadata
After this, there will be a PR for relating features with Lister/Detector to enable the API wise filtering. |
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.
only a nitpick and a question
worker.go
Outdated
return nil, err | ||
} | ||
|
||
log.WithFields( | ||
log.Fields{ |
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.
put this on the line above
j, _ := json.Marshal(m) | ||
json.Unmarshal(j, &c) | ||
return c | ||
} |
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.
Why don't we need this anymore?
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.
No, we don't need it. We will not compare crafted MetadataMap against MetadataMap from database query result any more because Updater will only compare MetadataMaps from database.
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.
https://github.com/coreos/clair/blob/master/updater.go#L515 It may be by design that we will only create notification when there's big change in the vulnerability. Maybe, in the future we can make it configurable.
435f672
to
e160616
Compare