Add truncindex to stores, to allow use of truncated IDs #6
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
runcom
reviewed
Dec 6, 2016
storage/layers.go
Outdated
| } | ||
| if layer, ok := r.byname[id]; ok { | ||
| id = layer.ID | ||
| } else if longid, err := r.idindex.Get(id); err == nil { |
Member
There was a problem hiding this comment.
can you have a function which fisrt check the r.byname[id] and the the truncindex to not repeat this else if over? skip this if you think it's not worth.
Member
Author
There was a problem hiding this comment.
Might as well; it should make a dent in the lint warnings.
Member
Author
|
Okay, updated. I think it got rid of a couple of lint warnings, at least. |
nalind
force-pushed
the
truncindex
branch
6 times, most recently
from
59bcd9d to
63fecb6
Compare
Add a truncindex to the layer, image, and container stores, so that layers, images, and containers can also be referred to by truncated versions of their IDs in any place where we previously accepted names. For containers, we don't add a truncated index for referring to containers by layer ID, because there doesn't appear to be a minimum length in truncindex, and we don't want to mistakenly treat a truncated but not sufficiently-unique container ID as a layer ID, which would likely resolve to some other different container. Signed-off-by: Nalin Dahyabhai <[email protected]>
Signed-off-by: Nalin Dahyabhai <[email protected]>
Signed-off-by: Nalin Dahyabhai <[email protected]>
Factor out name/ID lookups (ID as-is, name, top layer ID for containers, truncated ID) into a helper function to keep from duplicating them many times over. Factor out removeNames() helpers, too, since they all do the same thing. Signed-off-by: Nalin Dahyabhai <[email protected]>
Member
Author
|
Rebased on top of #24. |
Member
|
@runcom PTAL We need to get this in. |
Member
|
👍 |
Member
|
LGTM |
Member
|
@nalind Can we merge this now? |
Member
Author
|
Sure, merging. |
nalind
referenced
this pull request
in nalind/storage
Apr 19, 2017
Fix the Lookup methods in container and image stores, which broke when we merged #6. Signed-off-by: Nalin Dahyabhai <[email protected]>
nalind
referenced
this pull request
in nalind/storage
Apr 19, 2017
Fix canonicalization of layer IDs when computing layer differences, which we also broke in #6. Signed-off-by: Nalin Dahyabhai <[email protected]>
nalind
referenced
this pull request
in nalind/storage
Apr 19, 2017
Fix canonicalization of layer IDs when computing layer differences, which we also broke in #6. Signed-off-by: Nalin Dahyabhai <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add a truncindex to the layer, image, and container stores, so that layers, images, and containers can also be referred to by truncated versions of their IDs in any place where we previously accepted names.
For containers, we don't add a truncated index for referring to containers by layer ID, because there doesn't appear to be a minimum length in truncindex, and we don't want to mistakenly treat a truncated but not sufficiently-unique container ID as a layer ID, which would likely resolve to some other different container.