-
-
Notifications
You must be signed in to change notification settings - Fork 649
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
Docker image version tag #13097
Docker image version tag #13097
Conversation
…ions. # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
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.
This is neat! Not sure how generally useful it is to use the same version as a base image, but I can see uses for it.
) | ||
except (KeyError, ValueError) as e: | ||
msg = ( | ||
"Invalid format string for the `docker_image(version)` field: " |
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.
People might not know what you mean by "docker_image(version)
field". I'd go with "for the version
field of the docker_image target at {self.address.spec}`
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.
I don't have the address at that point, it is added further up the call stack. So the message becomes something like
"Error in {address}: Invalid format string for the version
field of the docker_image target."
OK? Otherwise I can pass the address to image_names()
for the sake of the error message..
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.
DockerBuildError: Error in testprojects/src/python/docker:test-example: Invalid format string for the `version` field of the docker_image target: '1.2.5-{typo}'.
The key 'typo' is unknown. Try with one of: baseimage, stage0.
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.
Doh. Dummy, of course I have the address... 🤦🏽
That is likely most useful only when mirroring images from one registry to another.. but then it helps avoiding typos! :D Plan is to add support for fishing out the |
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
[ci skip-rust]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
In order to keep the BUILD files DRY, support picking up the Docker image version to be built, from other sources rather than require a hard coded value in the BUILD file.
First up is support for looking at the
Dockerfile
FROM commands to glean possible version tags from them.Example:
FROM upstream:1.2-feat ...
This would then produce
myimage:1.2-feat
.As more format strings are introduced, so is the potential for users to shot them selves in the foot. So I've added some improved error feedback in case the version or image name template format strings blow up, that should help pinpoint the mistake and present possible solutions.