Skip to content

c8d/pull: Output truncated id for Pulling fs layer#47454

Merged
vvoland merged 1 commit intomoby:masterfrom
vvoland:c8d-pull-pullingfslayer-truncated
Feb 27, 2024
Merged

c8d/pull: Output truncated id for Pulling fs layer#47454
vvoland merged 1 commit intomoby:masterfrom
vvoland:c8d-pull-pullingfslayer-truncated

Conversation

@vvoland
Copy link
Contributor

@vvoland vvoland commented Feb 27, 2024

All other progress updates are emitted with truncated id.

$ docker pull --platform linux/amd64 alpine
Using default tag: latest
latest: Pulling from library/alpine
-sha256:4abcf20661432fb2d719aaf90656f55c287f8ca915dc1c92ec14ff61e67fbaf8: Pulling fs layer
+4abcf2066143: Download complete
Digest: sha256:c5b1261d6d3e43071626931fc004f70149baeba2c8ec672bd4f27761f8e1ad6b Status: Image is up to date for alpine:latest
docker.io/library/alpine:latest

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@vvoland vvoland added area/images Image Distribution containerd-integration Issues and PRs related to containerd integration area/ux labels Feb 27, 2024
@vvoland vvoland added this to the 26.0.0 milestone Feb 27, 2024
@vvoland vvoland self-assigned this Feb 27, 2024
All other progress updates are emitted with truncated id.

```diff
$ docker pull --platform linux/amd64 alpine
Using default tag: latest
latest: Pulling from library/alpine
-sha256:4abcf20661432fb2d719aaf90656f55c287f8ca915dc1c92ec14ff61e67fbaf8: Pulling fs layer
+4abcf2066143: Download complete
Digest: sha256:c5b1261d6d3e43071626931fc004f70149baeba2c8ec672bd4f27761f8e1ad6b
Status: Image is up to date for alpine:latest
docker.io/library/alpine:latest
```

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@vvoland vvoland force-pushed the c8d-pull-pullingfslayer-truncated branch from b78baf9 to 16aa7dd Compare February 27, 2024 10:09
Copy link
Contributor

@krissetto krissetto left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +122 to 124
id := stringid.TruncateID(desc.Digest.String())
progress.Update(out, id, "Pulling fs layer")
}
Copy link
Member

Choose a reason for hiding this comment

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

We really need to have a close look at these progress things. This should (really) be all part of the presentation, not the API. I realise we can't easily change all of that right now, so for now, it's probably OK to align with the existing behavior, but it's really nasty.

Also see;

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM 😢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/images Image Distribution area/ux containerd-integration Issues and PRs related to containerd integration status/4-merge

Projects

Development

Successfully merging this pull request may close these issues.

5 participants