Fix progress percent overflow in image push#153
Conversation
On certain scenarios, Docker will report a bigger Current than Total, resulting in an overflow when trying to show the progress of the push for the image. Cap progress percentage at 100 to prevent index out of bounds errors in progress display, as sometimes the calculation could exceed 100.
|
Hola! Encountered this issue while working on #152. In some cases I would randomly get the following overflow: This change solves that. Not the smartest or cleanest, but definitely the pragmatic one 😆 Thank you! |
psviderski
left a comment
There was a problem hiding this comment.
Thank you for taking your time to investigate the issue and implementing the fix.
Legend! 💯
| if jm.Progress.Total > 0 { | ||
| percent = int(jm.Progress.Current * 100 / jm.Progress.Total) | ||
| // Cap percent at 100 to prevent index out of bounds in progress display. | ||
| // Docker can report Current > Total in some cases (e.g., compression). |
There was a problem hiding this comment.
Ah wow, didn't expect that from docker 😆. Thank you for the fix! ❤️
There was a problem hiding this comment.
I was a bit surprised when saw this 🤣
There was a problem hiding this comment.
What also surprised is that Docker Compose doesn't have this check and my implementation was based on this: https://github.com/docker/compose/blob/main/pkg/compose/push.go#L130-L170
Maybe there is something specific in the unregistry implementation that triggered this 🤷
Anyway, I like the fix and the progress bar correctness is not something I'd like to spend my time on haha 😄
There was a problem hiding this comment.
Honestly in all my years, I've never used docker compose push, so I don't know 🫣
Let's keep it that way 😂
On certain scenarios, Docker will report a bigger Current than Total, resulting in an overflow when trying to show the progress of the push for the image.
Cap progress percentage at 100 to prevent index out of bounds errors in progress display, as sometimes the calculation could exceed 100.