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: Use cobra.Command.OutOrStdout method for output #1288

Merged
merged 2 commits into from
Mar 18, 2024

Conversation

TerryHowe
Copy link
Member

What this PR does / why we need it:

This contains the command refactor I did in another PR.

With this change, the output of a command can be set to something other than stdout. For example capture the output of a command:

    var stringWriter strings.Builder
    cmd := root.New()
    cmd.SetArgs([]string{"repo", "ls", "mcr.microsoft.com"})
    cmd.SetOutput(&stringWriter)
    _ := cmd.Execute()
    fmt.Println(stringWriter.String())

Copy link

codecov bot commented Mar 13, 2024

Codecov Report

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

Project coverage is 82.04%. Comparing base (f1d319f) to head (fe953b1).

Files Patch % Lines
cmd/oras/root/login.go 66.66% 1 Missing and 2 partials ⚠️
cmd/oras/root/discover.go 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1288      +/-   ##
==========================================
+ Coverage   81.94%   82.04%   +0.09%     
==========================================
  Files          83       83              
  Lines        4005     4016      +11     
==========================================
+ Hits         3282     3295      +13     
+ Misses        500      498       -2     
  Partials      223      223              

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

@qweeah
Copy link
Contributor

qweeah commented Mar 14, 2024

Need to confirm which output should be covered in this PR.

The proposed change in this PR doesn't cover all output of ORAS. E.g. the output of oras push/attach --format xxx which is designed to be used for automation. (There are also undergoing efforts for other commands, see #638) We have a draft doc https://hackmd.io/@shizh/HyFZkEcca to categorize all types of output in ORAS.

@TerryHowe Looks like you are trying to do automation and the target output to be changed is metadata and raw output?

@FeynmanZhou
Copy link
Member

Thanks @TerryHowe . This is a big PR. From users' perspective, I am curious on what's the impact to ORAS CLI output. There is not an issue associated with this PR to describe the problem or enhancement.

@TerryHowe TerryHowe changed the title Cmd out Use cobra.Command.OutOrStdout method for output Mar 14, 2024
@TerryHowe
Copy link
Member Author

TerryHowe commented Mar 14, 2024

This change has no effect on the format of the output and from my understanding no effect on CLI output. The change is really just for go scripting.

  • This change uses Fprintln instead of Println so we can pass a writer.
  • For the CLI cmd.OutOrStdout() will always return os.Stdout so no change in behavior

@TerryHowe
Copy link
Member Author

The goal is to allow oras go scripting to be able to capture output from commands without messing with stdout

@TerryHowe TerryHowe changed the title Use cobra.Command.OutOrStdout method for output feat: Use cobra.Command.OutOrStdout method for output Mar 14, 2024
@qweeah
Copy link
Contributor

qweeah commented Mar 15, 2024

The goal is to allow oras go scripting to be able to capture output from commands without messing with stdout

The output of --format won't go into STDOUT, e.g. localhost:5000/test@sha256:16ce5d4fb98496ec805e6b2401213598e710dbd936fb58cb7a325d2582924694 cannot captured by the go script.

> oras version
Version:        1.2.0-beta.1
Go version:     go1.21.6
Git commit:     9ffdb3eec60b969d842af1a9e699202e0827fa01
Git tree state: clean
> oras push localhost:5000/test:format --format {{.Ref}}
✓ Uploaded  application/vnd.oci.empty.v1+json                                                      2/2  B 100.00%   37ms
  └─ sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a
✓ Uploaded  application/vnd.oci.image.manifest.v1+json                                         535/535  B 100.00%     0s
  └─ sha256:16ce5d4fb98496ec805e6b2401213598e710dbd936fb58cb7a325d2582924694
localhost:5000/test@sha256:16ce5d4fb98496ec805e6b2401213598e710dbd936fb58cb7a325d2582924694

Signed-off-by: Terry Howe <tlhowe@amazon.com>
@TerryHowe
Copy link
Member Author

The output of --format won't go into STDOUT, e.g. localhost:5000/test@sha256:16ce5d4fb98496ec805e6b2401213598e710dbd936fb58cb7a325d2582924694 cannot captured by the go script.

I'd sooner handle format as a follow up.

Copy link
Contributor

@qweeah qweeah left a comment

Choose a reason for hiding this comment

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

LGTM

@qweeah qweeah merged commit bdc5696 into oras-project:main Mar 18, 2024
8 checks passed
@TerryHowe TerryHowe deleted the cmd-out branch May 5, 2024 14:34
FeynmanZhou pushed a commit to FeynmanZhou/oras that referenced this pull request May 11, 2024
)

Signed-off-by: Terry Howe <tlhowe@amazon.com>
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