[Splited commit]: tools/refenginediscovery: Refactor Ref-Engine Discovery library API #33
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
xiekeyang
changed the title
Pull the UI portions of this out into tools/cmd, because the library
API should not depend on urfave/cli.
The new refenginediscovery API is based on a per-configured-ref-engine
callback, which gives callers lots of flexibility in how they want to
handle each ref-engine and if/when they want to abort the
protocol/host iteration.
I've also adjusted the ref-engine interface to return a new MerkleRoot
type. This improves on the old Descriptor returns by:
* Being less opinionated about the root object, to stay in keeping
with ref-engine-discovery.md's:
The Merkle root may be a descriptor (media type
application/vnd.oci.descriptor.v1+json), or it may have a
different media type.
* Including the retrieval URI, which may be needed to expand
references in the root object (e.g. if the root has a casEngines
configuration which includes a relative reference).
These changes bring the API and output of the 'oci-discovery resolve
...' command much closer to the current Python module. The remaining
differences are the presence of a 'roots' indirection in the Python
output (currently used to pass through ref-engines-object-level
casEngines entries) and the lack of 'uri' entries for each root (which
I'm planning on adding).
Signed-off-by: xiekeyang <[email protected]>
Signed-off-by: xiekeyang <[email protected]>
wking
reviewed
Sep 21, 2017
| label: "manifests is not a JSON array", | ||
| response: `{"manifests": {}}`, | ||
| expected: "json: cannot unmarshal object into Go value of type []v1.Descriptor", | ||
| expected: "json: cannot unmarshal object into Go struct field Index.manifests of type []v1.Descriptor", |
Contributor
There was a problem hiding this comment.
I think these are due to changing versions (I developed the tests with Go 1.7.4). We probably want to use regexps instead of fixed strings. I'll put that into a rerolled #29.
Contributor
There was a problem hiding this comment.
We probably want to use regexps instead of fixed strings.
Done.
wking
reviewed
Sep 21, 2017
| } | ||
|
|
||
| uri, err := url.Parse("https://example.com/.well-known/oci-host-ref-engines") | ||
| uri, err := url.Parse("https://" + host + "/.well-known/oci-host-ref-engines") |
Contributor
There was a problem hiding this comment.
The previous approach avoided this unnecessary string concatenation. And once we add DNS-ancestor walking, we'll be setting uri.Host anyway.
wking
reviewed
Sep 21, 2017
| response := &http.Response{ | ||
| Request: request, | ||
| Body: ioutil.NopCloser(bytes.NewReader(bodyBytes)), | ||
| Body: ioutil.NopCloser(bytes.NewReader(bodyBytes)), |
Contributor
There was a problem hiding this comment.
Sorry for the sloppy commit. I've run go fmt … and squashed the fixes in here.
wking
reviewed
Sep 21, 2017
| } | ||
|
|
||
| assert.Equal(t, err.Error(), testcase.expected) | ||
| assert.Equal(t, testcase.expected, err.Error()) |
Contributor
There was a problem hiding this comment.
Good catch. Addressed this and other instances here.
Owner
Author
|
Closed by #29 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
split commit from #29.