Consistent UX for pull and push

[doringeman]

I would like the UX to be consistent.

$ ldm pull smollm2
bf6f20a60305: Pull complete [==================================================>]  270.6MB/270.6MB
cfc7749b96f6: Pull complete [==================================================>]  11.36kB/11.36kB
Model pulled successfully

$ ldm push doringeman657/smollm2
uploading: Push complete [==================================================>]  270.6MB/270.6MB

Model pushed successfully

Note the layer ID instead of "uploading", per-layer update vs single line and no blank line before the "Model pulled successfully" message.

[ericcurtin]

@jwjohns this might be an interesting one also

[jwjohns]

@ericcurtin I will take a look

[MetsysEht]

@doringeman I looked into the UX inconsistencies and found the following:

Extra blank line before success message
The blank line before Model pulled successfully comes from an extra println in cmd/cli/commands/push.go.
To keep the UX consistent, we should either:

1. remove the extra newline from push.go, or
2. add the same newline behavior to cmd/cli/commands/pull.go

Removing it from push.go seems cleaner since the pull output already looks correct.

uploading shown instead of layer ID during push
During push, the progress system uses a sentinel layer ID which is set to "uploading" defined in pkg/distribution/oci/progress.go.

This is intentional: unlike pull, push does not have a real layer ID available at the start, so "uploading" is used to distinguish uploads from downloads.

This is my first OS contribution, do let me know If I should raise a PR.

[ilopezluna]

Hello @jwjohns and @MetsysEht ! registry/client.go creates the reporter here.
To be able to show the layer ID and display a progress bar per layer, we would need to create a reporter per layer. You can see how we do this during pulling here:

model-runner/pkg/distribution/internal/store/store.go

Line 328 in 5a64bad

pr = progress.NewProgressReporter(safeWriter, progress.PullMsg, imageSize, l)

This is my first OS contribution, do let me know If I should raise a PR.

Yes please 🙏 PRs are welcome :)

[MetsysEht]

#Hello @doringeman and @ilopezluna . I have fixed the small issue of the extra line, Here is the PR
#566

As for the layerID issue, My first doubt is that does the layer even exists for it to have an id? As we do a nil check as well when pulling

model-runner/pkg/distribution/internal/progress/reporter.go

Line 87 in 5a64bad

if r.layer != nil { // In case of Pull

Also I can see in a comment in file

model-runner/pkg/distribution/oci/progress.go

Line 4 in 5a64bad

// During push, there is no real layer available yet, so this fake ID signals