-
Notifications
You must be signed in to change notification settings - Fork 66
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.
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
Skip docker push
if image exists remotely
#36
Conversation
Codecov Report
@@ Coverage Diff @@
## master #36 +/- ##
==========================================
+ Coverage 47.60% 48.12% +0.51%
==========================================
Files 27 27
Lines 2836 2849 +13
==========================================
+ Hits 1350 1371 +21
+ Misses 1486 1478 -8
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just a couple of questions. This is a nice upgrade which users will notice immediately.
|
||
def _image_tag_for_project(project_id: str, image_id: str) -> str: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would having a tag
parameter here that defaults to latest
be more flexible than just include_tag
? We don't really tag things differently now, but perhaps in the future.
|
||
""" | ||
image_tag = _image_tag_for_project(project_id, image_id, include_tag=False) | ||
cmd = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we pass the --project
flag here to the gcloud command, as we have the project_id
handy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIce!
This PR:
caliban.util.auth
andcaliban.docker.push
. 100% coverage on the latter, woohoo!docker push
. This saves some nice time during submission.I'll point this PR back at master once we merge #32; but this is based on the project refactor, so it looks cleanest when I point there for now.