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

refactor: add printer for metadata output #1310

Closed
wants to merge 1 commit into from

Conversation

qweeah
Copy link
Contributor

@qweeah qweeah commented Mar 26, 2024

What this PR does / why we need it:
This PR adds a printer so user-specified cmd output in go scripting works as expected.

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Copy link

codecov bot commented Mar 26, 2024

Codecov Report

Attention: Patch coverage is 91.07143% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 82.10%. Comparing base (776f8df) to head (d99475b).

Files Patch % Lines
cmd/oras/internal/display/metadata/view/printer.go 72.22% 4 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1310      +/-   ##
==========================================
+ Coverage   81.99%   82.10%   +0.10%     
==========================================
  Files          83       82       -1     
  Lines        4016     4040      +24     
==========================================
+ Hits         3293     3317      +24     
  Misses        500      500              
  Partials      223      223              

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

type Printer interface {
Printf(format string, a ...any) (n int, err error)
Println(a ...any) (n int, err error)
PrintJSON(object any) error
Copy link
Member

Choose a reason for hiding this comment

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

I was thinking the printer wouldn't know about format, but I'll have to think about that more.

I was in the process of unwinding verbose everywhere which is a lot more complicated than this.

Copy link
Contributor Author

@qweeah qweeah Mar 27, 2024

Choose a reason for hiding this comment

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

verbose serves only for status output. E.g. in the below output:

output oras push localhost:5000/demo:output image.tar --no-tty
Uploading 8b138a688d47 image.tar
Uploaded  8b138a688d47 image.tar
Pushed [registry] localhost:5000/demo:output
Digest: sha256:b7593bbc81769aa9fef0b37b73405eccfaf968a56e6b542614cfb19fe376e782

The status output is below and only covers named content

Uploading 8b138a688d47 image.tar
Uploaded  8b138a688d47 image.tar

If we run it in verbose mode, then the output is

output oras push localhost:5000/demo:output image.tar --no-tty --verbose
Preparing image.tar
Uploading 8b138a688d47 image.tar
Uploading 44136fa355b3 application/vnd.oci.empty.v1+json
Uploaded  8b138a688d47 image.tar
Uploaded  44136fa355b3 application/vnd.oci.empty.v1+json
Uploading 262c1a7d7850 application/vnd.oci.image.manifest.v1+json
Uploaded  262c1a7d7850 application/vnd.oci.image.manifest.v1+json
Pushed [registry] localhost:5000/demo:output
Digest: sha256:262c1a7d7850ee622859898dafa7a13618fabb9beb5b4fa5dfb8cc1e73bbb8d7

In this case, the status output that covers unnamed content is:

Uploading 8b138a688d47 image.tar
Uploading 44136fa355b3 application/vnd.oci.empty.v1+json
Uploaded  8b138a688d47 image.tar
Uploaded  44136fa355b3 application/vnd.oci.empty.v1+json
Uploading 262c1a7d7850 application/vnd.oci.image.manifest.v1+json
Uploaded  262c1a7d7850 application/vnd.oci.image.manifest.v1+json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR only focus on metadata output, in the above case, the metadata output is:

Pushed [registry] localhost:5000/demo:output
Digest: sha256:262c1a7d7850ee622859898dafa7a13618fabb9beb5b4fa5dfb8cc1e73bbb8d7

and

Pushed [registry] localhost:5000/demo:output
Digest: sha256:b7593bbc81769aa9fef0b37b73405eccfaf968a56e6b542614cfb19fe376e782

@qweeah
Copy link
Contributor Author

qweeah commented Mar 27, 2024

Discussed offline with @shizhMSFT, closing it and will refactor in more straightforward way.

@qweeah qweeah closed this Mar 27, 2024
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.

2 participants