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

Check media type of manifest before fallback to OCI image for oras push and oras atatch #672

Closed
qweeah opened this issue Nov 4, 2022 · 2 comments · Fixed by #673
Closed

Comments

@qweeah
Copy link
Contributor

qweeah commented Nov 4, 2022

    Curious why we chose names that result in double-negatives?  For example *"is it not not-supported"* vs *"is it supported"*.

Originally posted by @nima in #665 (comment)

@qweeah qweeah changed the title Improve reability of OCI artifact fallback code Improve code reability of OCI artifact push fallback Nov 4, 2022
@qweeah
Copy link
Contributor Author

qweeah commented Nov 4, 2022

We should also rename the function to NoFallbackToOciImage or something indicates that we should not retry with OCI Image?

The reason is both isArtifactUnsupported and isArtifactSupported is not accurate, we don't know if artifact is supported if the other HTTP error happens.

@qweeah
Copy link
Contributor Author

qweeah commented Nov 4, 2022

@nima @sajayantony After talking to @shizhMSFT I found that we should not change !isArtifactUnsupported to isArtifactSupported since we are really checking an Unsupported error code, here is a golang blog that explains in detail.

Still thanks for raising this, it helps me I found that we still need to enhance the check for oci artifact media type. I had a PR #673 for it. Can you help review, thanks.

@qweeah qweeah changed the title Improve code reability of OCI artifact push fallback Check media type of manifest before fallback to OCI image for oras push and oras atatch Nov 4, 2022
TerryHowe pushed a commit to TerryHowe/oras that referenced this issue Feb 2, 2023
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 a pull request may close this issue.

1 participant