-
Couldn't load subscription status.
- Fork 4
Feat add in build tool control to config #35
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
Feat add in build tool control to config #35
Conversation
… control over the pod-names.
…eat-adjust-pod-name-prefix
…eat-add-in-build-tool-control-to-config
| if not is_command_available(f"sudo {BuildTool.podman.value}"): | ||
| raise Exception("Podman installation is required to run dagster-uc.") | ||
| else: | ||
| raise Warning("Sudo is required to run podman") |
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 not true, you can run podman fine without sudo
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.
Apparently not in Azure Pipelines! Either that or I have to do some funkier stuff to make it work. Running podman info fails right after installation, but sudo podman info works fine.
|
A while I removed docker support to simplify the maintenance of the project. Additionally the way it's currently reintroduce will causes issues because fetching tags from the repository is only done using podman at the moment. |
Understood. i have removed that from the PR, I left in the build format as that may be useful later on. |
dagster_uc/utils.py
Outdated
| if build_tool == BuildTool.podman.value and build_format == "docker": | ||
| 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.
you can simplify this by making just doing:
if build_tool == BuildTool.podman.value and build_format == "docker":
cmd += ["--format", "docker"]
| typer.echo(f"Logging into acr with {BuildTool.podman.value}...") | ||
| token = get_azure_access_token(image_registry) | ||
| cmd = [ | ||
| BuildTool.podman.value, |
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 would keep BuildTool.podman.value
| os.chdir(repository_root) | ||
|
|
||
| cmd = [ | ||
| BuildTool.podman.value, |
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 would keep BuildTool.podman.value
Add in the ability to control build tool for CICD purposes as well as output format (some tools only allow docker specific images) via the config file.