Skip to content

Commit

Permalink
resolve comments
Browse files Browse the repository at this point in the history
Signed-off-by: Billy Zha <[email protected]>
  • Loading branch information
qweeah committed Mar 25, 2024
1 parent 63d664b commit a83297b
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 33 deletions.
6 changes: 2 additions & 4 deletions cmd/oras/internal/display/status/discard.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ limitations under the License.
package status

import (
"sync"

ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"oras.land/oras-go/v2"
"oras.land/oras-go/v2/content"
Expand Down Expand Up @@ -65,7 +63,7 @@ func (DiscardHandler) OnNodeDownloaded(desc ocispec.Descriptor) error {
}

// OnNodeRestored implements PullHandler.
func (DiscardHandler) OnNodeRestored(_ *sync.Map, _ ocispec.Descriptor) error {
func (DiscardHandler) OnNodeRestored(_ ocispec.Descriptor) error {
return nil
}

Expand All @@ -75,6 +73,6 @@ func (DiscardHandler) OnNodeProcessing(desc ocispec.Descriptor) error {
}

// OnNodeProcessing implements PullHandler.
func (DiscardHandler) OnNodeSkipped(printed *sync.Map, desc ocispec.Descriptor) error {
func (DiscardHandler) OnNodeSkipped(desc ocispec.Descriptor) error {
return nil

Check warning on line 77 in cmd/oras/internal/display/status/discard.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/status/discard.go#L76-L77

Added lines #L76 - L77 were not covered by tests
}
5 changes: 2 additions & 3 deletions cmd/oras/internal/display/status/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package status

import (
"io"
"sync"

ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"oras.land/oras-go/v2"
Expand Down Expand Up @@ -48,7 +47,7 @@ type PullHandler interface {
// OnNodeDownloaded is called after a node is downloaded.
OnNodeDownloaded(desc ocispec.Descriptor) error
// OnNodeRestored is called after a deduplicated node is restored.
OnNodeRestored(printed *sync.Map, desc ocispec.Descriptor) error
OnNodeRestored(desc ocispec.Descriptor) error
// OnNodeSkipped is called when a node is skipped.
OnNodeSkipped(printed *sync.Map, desc ocispec.Descriptor) error
OnNodeSkipped(desc ocispec.Descriptor) error
}
15 changes: 4 additions & 11 deletions cmd/oras/internal/display/status/text.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,6 @@ type TextPullHandler struct {
verbose bool
}

func (ph *TextPullHandler) printOnce(printed *sync.Map, s ocispec.Descriptor, msg string) error {
if _, loaded := printed.LoadOrStore(utils.GenerateContentKey(s), true); loaded {
return nil
}
return PrintStatus(s, msg, ph.verbose)
}

// TrackTarget returns a tracked target.
func (ph *TextPullHandler) TrackTarget(gt oras.GraphTarget) (oras.GraphTarget, error) {
ph.fetcher = gt
Expand All @@ -123,8 +116,8 @@ func (ph *TextPullHandler) OnNodeDownloaded(desc ocispec.Descriptor) error {
}

// OnNodeRestored implements PullHandler.
func (ph *TextPullHandler) OnNodeRestored(printed *sync.Map, desc ocispec.Descriptor) error {
return ph.printOnce(printed, desc, utils.PullPromptRestored)
func (ph *TextPullHandler) OnNodeRestored(desc ocispec.Descriptor) error {
return PrintStatus(desc, utils.PullPromptRestored, ph.verbose)
}

// OnNodeProcessing implements PullHandler.
Expand All @@ -133,8 +126,8 @@ func (ph *TextPullHandler) OnNodeProcessing(desc ocispec.Descriptor) error {
}

// OnNodeProcessing implements PullHandler.
func (ph *TextPullHandler) OnNodeSkipped(printed *sync.Map, desc ocispec.Descriptor) error {
return ph.printOnce(printed, desc, utils.PullPromptSkipped)
func (ph *TextPullHandler) OnNodeSkipped(desc ocispec.Descriptor) error {
return PrintStatus(desc, utils.PullPromptSkipped, ph.verbose)
}

// NewTextPullHandler returns a new handler for pull command.
Expand Down
15 changes: 4 additions & 11 deletions cmd/oras/internal/display/status/tty.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,6 @@ func NewTTYPullHandler(tty *os.File) PullHandler {
}
}

func (ph *TTYPullHandler) printOnce(printed *sync.Map, s ocispec.Descriptor, msg string) error {
if _, loaded := printed.LoadOrStore(utils.GenerateContentKey(s), true); loaded {
return nil
}
return ph.tracked.Prompt(s, msg)
}

// OnNodeDownloading implements PullHandler.
func (ph *TTYPullHandler) OnNodeDownloading(desc ocispec.Descriptor) error {
return nil
Expand All @@ -125,13 +118,13 @@ func (ph *TTYPullHandler) OnNodeProcessing(desc ocispec.Descriptor) error {
}

// OnNodeRestored implements PullHandler.
func (ph *TTYPullHandler) OnNodeRestored(printed *sync.Map, desc ocispec.Descriptor) error {
return ph.printOnce(printed, desc, "Restored ")
func (ph *TTYPullHandler) OnNodeRestored(desc ocispec.Descriptor) error {
return ph.tracked.Prompt(desc, utils.PullPromptRestored)

Check warning on line 122 in cmd/oras/internal/display/status/tty.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/status/tty.go#L121-L122

Added lines #L121 - L122 were not covered by tests
}

// OnNodeProcessing implements PullHandler.
func (ph *TTYPullHandler) OnNodeSkipped(printed *sync.Map, desc ocispec.Descriptor) error {
return ph.printOnce(printed, desc, "Skipped ")
func (ph *TTYPullHandler) OnNodeSkipped(desc ocispec.Descriptor) error {
return ph.tracked.Prompt(desc, utils.PullPromptSkipped)

Check warning on line 127 in cmd/oras/internal/display/status/tty.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/status/tty.go#L126-L127

Added lines #L126 - L127 were not covered by tests
}

// Close implements io.Closer.
Expand Down
12 changes: 8 additions & 4 deletions cmd/oras/root/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,10 @@ func doPull(ctx context.Context, src oras.ReadOnlyTarget, dst oras.GraphTarget,
}
if len(ss) == 0 {
// skip s if it is unnamed AND has no successors.
if err := statusHandler.OnNodeSkipped(&printed, s); err != nil {
return nil, err
if _, loaded := printed.LoadOrStore(utils.GenerateContentKey(s), true); !loaded {
if err := statusHandler.OnNodeSkipped(s); err != nil {
return nil, err
}

Check warning on line 232 in cmd/oras/root/pull.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/root/pull.go#L231-L232

Added lines #L231 - L232 were not covered by tests
}
continue
}
Expand All @@ -254,8 +256,10 @@ func doPull(ctx context.Context, src oras.ReadOnlyTarget, dst oras.GraphTarget,
filesLock.Lock()
files = append(files, model.NewFile(name, po.Output, s, po.Path))
filesLock.Unlock()
if err := statusHandler.OnNodeRestored(&printed, s); err != nil {
return err
if _, loaded := printed.LoadOrStore(utils.GenerateContentKey(s), true); !loaded {
if err := statusHandler.OnNodeRestored(s); err != nil {
return err
}

Check warning on line 262 in cmd/oras/root/pull.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/root/pull.go#L261-L262

Added lines #L261 - L262 were not covered by tests
}
}
}
Expand Down

0 comments on commit a83297b

Please sign in to comment.