Tool to get image digest #87
Conversation
|
@mattmoor could you take a look please? |
|
@matmoor any news? (just noticed the misspell) |
|
@dekkagaijin can you take a look? |
|
@mattmoor any news? |
|
@ash2k the build is failing btw |
|
@deft-code @KaylaNguyen PTAL |
|
@ittaiz the build got stuck because of docker login it seems. I cannot do anything about it, someone needs to push a button to restart it. |
|
I think you can close and then reopen the PR and it will re-trigger.
Another option is an empty commit.
…On Thu, 19 Jul 2018 at 4:35 Mikhail Mazurskiy ***@***.***> wrote:
@ittaiz <https://github.com/ittaiz> the build got stuck because of docker
login it seems. I cannot do anything about it, someone needs to push a
button to restart it.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#87 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIFz6y-QDssc4O2W-tuzHBhYGXzglnks5uH-JkgaJpZM4VDVaS>
.
|
KaylaNguyen
left a comment
KaylaNguyen
left a comment
There was a problem hiding this comment.
Sorry for the long delay! Thanks for being patient with us.
| parser = argparse.ArgumentParser( | ||
| description='Calculate digest for a container image.') | ||
|
|
||
| parser.add_argument( |
There was a problem hiding this comment.
Please specify types and whether is required for these flag values.
There was a problem hiding this comment.
They are all strings, except for the --oci which is boolean and action='store_true' ensures it is either true or false. Is that ok or I need to explicitly put all types there? Please note that I've just copied fast_pusher_.py and removed what is not needed, following the style as close as possible. It does not have types or required markers.
There was a problem hiding this comment.
It's up to you at this point. I'm happy with what you have now. You don't have to follow fast_pusher_.py code style.
| logging.info('Reading config from tarball %r', args.tarball) | ||
| with v2_2_image.FromTarball(args.tarball) as base: | ||
| config = base.config_file() | ||
|
|
There was a problem hiding this comment.
Is tarball guaranteed to exists? What would happen if neither config and tarball exists?
There was a problem hiding this comment.
There is/was a check above that terminated the program if neither config nor tarball parameters are provided. I moved it here into an else branch. Reads better now. (this is originally copied from another file too).
|
Travis blocks on docker login each time. 🤷♂️ |
|
Can you try what @ittaiz mentioned to see if that fixes the docker login problem? |
|
The root cause of the |
|
I'm reluctant to test and merge this PR on github, as internal google code is currently the source of truth and changing code externally would make internal and external container registry code diverge. |
|
@KaylaNguyen I'm happy with anything as long as this code will get into this repo. |
|
Resolving :) |
|
@KaylaNguyen any updates? |
|
I'm working on it. Out for review, pending adding test. |
|
The change was rolled out. I'm closing this PR :) |
|
@KaylaNguyen thanks a lot! |
|
Awesome!
…On Thu, 23 Aug 2018 at 8:37 Mikhail Mazurskiy ***@***.***> wrote:
@KaylaNguyen <https://github.com/KaylaNguyen> thanks a lot!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#87 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIF3qlyzD_tU11Zn5qwC43OYvrEJnJks5uTj-VgaJpZM4VDVaS>
.
|
|
Would it be possible to cut a release so that I can use it in my rules_docker PR bazelbuild/rules_docker#445 ? Thanks! |
|
@ash2k Ack. There are two small changes I want to land before this release, but I'll try to cut it today 👍 |
|
@ash2k done now (thanks @KaylaNguyen!) |
6 participants
Used in bazelbuild/rules_docker#445.