Skip to content
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

implement publish #11008

Merged
merged 1 commit into from
Sep 20, 2023
Merged

implement publish #11008

merged 1 commit into from
Sep 20, 2023

Conversation

ndeloof
Copy link
Contributor

@ndeloof ndeloof commented Sep 15, 2023

What I did
implement publish command, to publish compose.yaml files as pseudo-image "layers" with a custom artifact type

@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.53% ⚠️

Comparison is base (a697a06) 57.93% compared to head (9b2675d) 57.40%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11008      +/-   ##
==========================================
- Coverage   57.93%   57.40%   -0.53%     
==========================================
  Files         129      129              
  Lines       11121    11223     +102     
==========================================
  Hits         6443     6443              
- Misses       4044     4146     +102     
  Partials      634      634              
Files Changed Coverage Δ
pkg/compose/publish.go 0.00% <0.00%> (ø)
pkg/remote/oci.go 2.38% <0.00%> (-0.09%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@milas milas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I bikeshed all the names but they also seem fine as-is.

One thing worth mentioning is that OCI projects won't work (with include) if they have external file dependencies like <svc>.build or <svc>.env_file, since it'll look in the temp/cache download directory when trying to load. I don't think that needs to block this, though

Digest: digest.FromString(string(f)),
Size: int64(len(f)),
Annotations: map[string]string{
"com.docker.compose": api.ComposeVersion,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

com.docker.compose.version perhaps?

})
layer := v1.Descriptor{
MediaType: v1.MediaTypeImageLayer,
Digest: digest.FromString(string(f)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Digest: digest.FromString(string(f)),
Digest: digest.FromBytes(f),

Internally, digest.FromString will just convert back to bytes so might as well pass it directly

Annotations: map[string]string{
"com.docker.compose": api.ComposeVersion,
},
ArtifactType: "application/vnd.docker.compose.file",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this should have a +yaml

Annotations: map[string]string{
"com.docker.compose": api.ComposeVersion,
},
ArtifactType: "application/vnd.docker.compose",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.project maybe?

@ndeloof ndeloof force-pushed the oci-publish branch 2 times, most recently from 07aa128 to ead0ce4 Compare September 17, 2023 19:26
Signed-off-by: Nicolas De Loof <[email protected]>
@ndeloof
Copy link
Contributor Author

ndeloof commented Sep 18, 2023

also adopted oci:// as a prefix to align with helm

@ndeloof ndeloof requested review from a team, nicksieger, StefanScherer, ulyssessouza and laurazard and removed request for a team September 18, 2023 12:33
Copy link
Contributor

@glours glours left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ndeloof ndeloof merged commit 5ca35c8 into docker:main Sep 20, 2023
25 checks passed
@ndeloof ndeloof deleted the oci-publish branch September 20, 2023 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants