Add configurable layers in manifest events #2639
Conversation
Signed-off-by: Andrew Leung <[email protected]>
|
Hi @dmcgowan , could you take a look please or refer the PR to who should take a look? Thanks! |
notifications/bridge.go
Outdated
| request RequestRecord | ||
| sink Sink | ||
| ub URLBuilder | ||
| manfiestLayers bool |
There was a problem hiding this comment.
typo: s/manfiestLayers/manifestLayers/
|
Please sign your commits following these rules: $ git clone -b "manifesteventlayers" [email protected]:andrew-leung/distribution.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354382608
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -fAmending updates the existing PR. You DO NOT need to open a new one. |
Signed-off-by: Andrew Leung <[email protected]>
andrew-leung
force-pushed
the
manifesteventlayers
branch
from
6993eb7
to
54aef62
Compare
Codecov Report
@@ Coverage Diff @@
## master #2639 +/- ##
==========================================
- Coverage 61.03% 51.16% -9.88%
==========================================
Files 132 129 -3
Lines 12122 11904 -218
==========================================
- Hits 7399 6091 -1308
- Misses 3803 5052 +1249
+ Partials 920 761 -159
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
This is great. I also have a need for this exact functionality. My only thought if it should be called References instead of Layers since that closely resembles the term used by registry. Also container config json returned by References() is not really a layer. Would live @dmcgowan opinion on this.
|
@manishtomar I agree |
Signed-off-by: Andrew Leung <[email protected]>
|
@manishtomar @dmcgowan , good suggestion. I've updated the terminology to use |
|
@manishtomar & @dmcgowan , please let me know if there is anything else needed prior to merging. |
|
LGTM |
|
@andrew-leung Sorry lost track of this. This looks good from my side. @dmcgowan @dmp42 Please take another look and if looks good please merge it. |
|
This change isn't great. It breaks the abstraction and pulls format specific items up to the event, which makes migration impossible. This will also making adding new formats much more challenging. Applications that need this information should just pull the manifest to get the references. Also, this should have incremented the version for the event mediatype. In the future, let's make sure we've exhausted all client-side options before adding new, format-specific functionality to the registry. |
Adds a configurable option to include layer info in manifest notification events. When we receive manifest notification events we often want to know which layers were accessed and/or compose the image. Rather than re-querying the registry, it is convenient to have that information in the manifest event itself.
With the change the default behavior of not including layer information in manifest events is not changed. An explicit configuration is needed to include the info.