add progress bar support for apptainer push/pull for oras protocol

[JasonYangShadow]

Description of the Pull Request (PR):

add progress bar support for apptainer push/pull for oras protocol

This fixes or addresses the following GitHub issues:

• Fixes Feature request: progress indicator during apptainer push #437

Before submitting a PR, make sure you have done the following:

• Read the Guidelines for Contributing, and this PR conforms to the stated requirements.
• Added changes to the CHANGELOG if necessary according to the Contribution Guidelines
• Added tests to validate this PR, linted with make check and tested this PR locally with a make test, and make testall if possible (see CONTRIBUTING.md).
• Based this PR against the appropriate branch according to the Contribution Guidelines
• Added myself as a contributor to the Contributors File

Test

Apptainer Push

Apptainer Pull

[panda1100]

I tested push/pull and both works great! Thank you @JasonYangShadow !

[DrDaveD]

I'm surprised this has a whole new source file just for the oras protocol. Aren't there progress bars on other protocols that can share code? Or is this the first one and it should be extended to others? Why should we treat the oras protocol specially?

[panda1100]

@DrDaveD -san,
This is not the first progress bar implementation. docker:// pull and library:// push/pull has progress bar. Each has its own implementation.
The reason we need unique implementation for oras "as well", as my understanding from discussing with @JasonYangShadow and @cclerget about this PR design, we need to access reader/writer buffer to track progress to implement progress bar, to access those buffer we need to control over the reader/writer returned by Push/Fetch method those are under oras library (those are not exposed as callable function?). oras library itself doesn't have progress bar implementation yet. so, we need to use those interfaces directly from Apptainer. This is the reason we decided to implement this way.

Please feel free to follow up or to correct me @JasonYangShadow and @cclerget .

[DrDaveD]

The reason we need unique implementation for oras "as well", as my understanding from discussing with @JasonYangShadow and @cclerget about this PR design, we need to access reader/writer buffer to track progress to implement progress bar, to access those buffer we need to control over the reader/writer returned by Push/Fetch method those are under oras library (those are not exposed as callable function?).

Are you saying the code can't be refactored to share at least the visual part of the progress bar implementation? That's not good for maintenance, because one implementation might get changed to improve something and the others forgotten.

[panda1100]

Thank you @DrDaveD -san, I see your point. I'll discuss with @JasonYangShadow about it tomorrow morning.

[JasonYangShadow]

I will take a look at current progressbar and mpb related implementation and see if we can reuse mpb.

[JasonYangShadow]

@panda1100 I have rewritten the function using apptainer existing progressbar, the same we used previously. Please help verify and check if it is okay.

[panda1100]

@JasonYangShadow Thank you Jason! I did test on my end and works good for both push and pull 👍

[DrDaveD]

This is an improvement. However I still don't like that there's a new function called getOrasResolver() along with an old function called getResolver() with the same parameters and only for oras, in fact in oras.go. It also looks like they're mostly identical code too.

I suggest renaming getOrasResolver to getResolver and adding a flag that makes it optionally behave like the existing getResolver, then remove the current getResolver.

[JasonYangShadow]

This is an improvement. However I still don't like that there's a new function called getOrasResolver() along with an old function called getResolver() with the same parameters and only for oras, in fact in oras.go. It also looks like they're mostly identical code too.

I suggest renaming getOrasResolver to getResolver and adding a flag that makes it optionally behave like the existing getResolver, then remove the current getResolver.

Thanks for the comments, next time I will be more serious on the duplicated code.

[DrDaveD]

Please also cherry-pick to release-1.2

[panda1100]

Thank you DrDave-san and Jason!