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

feat: adds media type detection when loading OCI descriptors #15

Merged
merged 1 commit into from
May 31, 2022

Conversation

jpower432
Copy link
Contributor

Closes #14

Signed-off-by: Jennifer Power barnabei.jennifer@gmail.com

Closes ortelius#14

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
@afflom
Copy link

afflom commented May 24, 2022

The warning was confusing:

WARN[0000] reference for unknown type: text/plain; charset=utf-8 
WARN[0000] reference for unknown type: image/jpeg       
WARN[0000] reference for unknown type: text/plain; charset=utf-8 
WARN[0000] reference for unknown type: application/vnd.uor.config.v1+json 
WARN[0000] reference for unknown type: application/json 
WARN[0000] reference for unknown type: image/jpeg       
WARN[0000] reference for unknown type: image/jpeg  

But the code runs as advertised!

@afflom afflom requested a review from adrezni May 24, 2022 20:50
@jpower432
Copy link
Contributor Author

The warning was confusing:

WARN[0000] reference for unknown type: text/plain; charset=utf-8 
WARN[0000] reference for unknown type: image/jpeg       
WARN[0000] reference for unknown type: text/plain; charset=utf-8 
WARN[0000] reference for unknown type: application/vnd.uor.config.v1+json 
WARN[0000] reference for unknown type: application/json 
WARN[0000] reference for unknown type: image/jpeg       
WARN[0000] reference for unknown type: image/jpeg  

But the code runs as advertised!

This seems to come from the containerd library used by oras. Looks like it appears anytime the media type is outside of the OCI Spec which for this purpose is expected. I think if the CopyOptions for oras were appended to allow these as allowedMediaTypes this warning may go away. It's worth trying.

@@ -5,13 +5,16 @@ import (
"fmt"
"path/filepath"

"github.com/gabriel-vasile/mimetype"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a specific need to bring in a 3rd party library when http.DetectContentType is available?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sabre1041 http.DetectContentType does not recognize JSON. We originally pulled this import in to detected content types in the parser package; so detected format types are more important here. Just decided to use the already imported library in this instance as well. https://github.com/uor-framework/client/blob/3c715c563642f7542e70799b4afcb43c1018c5e8/builder/parser/parser.go#L37.

@afflom afflom merged commit 80a297c into ortelius:main May 31, 2022
@jpower432 jpower432 deleted the feat/media-type-detection branch July 30, 2022 00:49
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.

[RFE] Detect the media type of files when generating the manifest
3 participants