Skip to content

Display image size for multiarch container images #23821

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Apr 2, 2023
Merged

Display image size for multiarch container images #23821

merged 4 commits into from
Apr 2, 2023

Conversation

KN4CK3R
Copy link
Member

@KN4CK3R KN4CK3R commented Mar 30, 2023

Fixes #23771

Changes the display of different architectures for multiarch images to show the image size:
grafik

@KN4CK3R KN4CK3R added type/enhancement An improvement of existing functionality topic/packages labels Mar 30, 2023
@KN4CK3R KN4CK3R added this to the 1.20.0 milestone Mar 30, 2023
@KN4CK3R KN4CK3R changed the title Change package container metadata Display image size for multiarch container images Mar 30, 2023
@codecov-commenter
Copy link

codecov-commenter commented Mar 30, 2023 •
edited
Loading

Codecov Report

Merging #23821 (7b61d2f) into main (f521e88) will decrease coverage by 0.14%.
The diff coverage is 28.82%.

@@            Coverage Diff             @@
##             main   #23821      +/-   ##
==========================================
- Coverage   47.14%   47.01%   -0.14%     
==========================================
  Files        1149     1158       +9     
  Lines      151446   153197    +1751     
==========================================
+ Hits        71397    72023     +626     
- Misses      71611    72674    +1063     
- Partials     8438     8500      +62
Impacted Files Coverage Δ
cmd/dump.go 0.66% <0.00%> (-0.01%) ⬇️
cmd/mailer.go 0.00% <0.00%> (ø)
cmd/manager.go 0.00% <0.00%> (ø)
cmd/manager_logging.go 0.00% <0.00%> (ø)
cmd/migrate_storage.go 5.76% <0.00%> (-0.12%) ⬇️
cmd/restore_repo.go 0.00% <0.00%> (ø)
cmd/web.go 0.00% <0.00%> (ø)
models/actions/run.go 1.63% <0.00%> (-0.10%) ⬇️
models/actions/runner.go 1.44% <ø> (ø)
models/packages/package.go 45.45% <0.00%> (-1.13%) ⬇️
... and 67 more

... and 74 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 30, 2023
@lunny
Copy link
Member

lunny commented Mar 30, 2023

Could we display the summmary size of all images in the right siderbar?

@KN4CK3R
Copy link
Member Author

KN4CK3R commented Mar 30, 2023

Could we display the summmary size of all images in the right siderbar?

I see no benefit doing that because that value is meaningless. The docker client downloads just the matching architecture and not every possible image.

@lunny
Copy link
Member

lunny commented Mar 31, 2023

But it's confusing the size is 770B on the right sidebar on your image. What's that size?

@KN4CK3R
Copy link
Member Author

KN4CK3R commented Mar 31, 2023

The image is a manifest list (a json file) which just references other images (which this PR lists with their size now). So the package you are looking at is small, 770B.

@lunny
Copy link
Member

lunny commented Mar 31, 2023

The image is a manifest list (a json file) which just references other images (which this PR lists with their size now). So the package you are looking at is small, 770B.

Yes, I know. That's why I think it's confusing. I think users don't care about the manifest's size because they know it's small. They should care about the total size of this package.

@KN4CK3R
Copy link
Member Author

KN4CK3R commented Mar 31, 2023

But there is no "total" size because a client will never download all of it (if you don't enforce it). How about not displaying the size if it's a manifest list?

@Zettat123
Copy link
Contributor

Zettat123 commented Mar 31, 2023 •
edited
Loading

I think it's not necessary to show the size (the size of manifest) for multi-arch images, just like Docker Hub.

@lunny
Copy link
Member

lunny commented Mar 31, 2023

I think it's not necessary to show the size (the size of manifest) for multi-arch images, just like Docker Hub.

Hide the manifest size is also OK for me to make it less confusing.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 1, 2023
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 1, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 1, 2023
@lunny lunny merged commit fbd4eac into go-gitea:main Apr 2, 2023
@lunny lunny removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 2, 2023
@KN4CK3R KN4CK3R deleted the feature-container-architectures branch April 2, 2023 09:59
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 3, 2023
@zjjhot
Merge remote-tracking branch 'upstream/main'
a6741d8 
* upstream/main:
  [skip ci] Updated translations via Crowdin
  Update JS deps (go-gitea#23853)
  Added close/open button to details page of milestone (go-gitea#23877)
  Check `IsActionsToken` for LFS authentication (go-gitea#23841)
  Prefill input values in oauth settings as intended (go-gitea#23829)
  Display image size for multiarch container images (go-gitea#23821)
  Use clippie module to copy to clipboard (go-gitea#23801)
  Remove assertion debug code for show/hide refactoring (go-gitea#23576)
  [skip ci] Updated translations via Crowdin
  Remove jQuery ready usage (go-gitea#23858)
  Fix JS error when changing PR's target branch (go-gitea#23862)
  Improve action log display with control chars (go-gitea#23820)
  Fix review conversation reply (go-gitea#23846)
  Improve home page template, fix Sort dropdown menu flash (go-gitea#23856)
  Make first section on home page full width (go-gitea#23854)
  [skip ci] Updated translations via Crowdin
  Fix incorrect CORS failure detection logic (go-gitea#23844)
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lunny lunny lunny approved these changes

+1 more reviewer

@Zettat123 Zettat123 Zettat123 approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/packages type/enhancement An improvement of existing functionality
Projects
None yet
Milestone
1.20.0
Development

Successfully merging this pull request may close these issues.

Reported container image size on UI seems incorrect
5 participants
@KN4CK3R @codecov-commenter @lunny @Zettat123 @GiteaBot