Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add config option #294

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add config option #294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6f69add
Added config option to pass arguments as a yaml file instead
agyoungs Oct 30, 2024
0f45fb2
Updated help text
agyoungs Oct 30, 2024
397672a
Formatting
agyoungs Oct 30, 2024
4182cf4
Added missing dep
agyoungs Oct 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
'pexpect',
'packaging',
'urllib3',
'pyyaml',
]

# docker API used to be in a package called `docker-py` before the 2.0 release
Expand Down
11 changes: 11 additions & 0 deletions src/rocker/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import argparse
import os
import sys
import yaml

from .core import DockerImageGenerator
from .core import get_rocker_version
Expand All @@ -32,6 +33,10 @@ def main():
formatter_class=argparse.ArgumentDefaultsHelpFormatter)
parser.add_argument('image')
parser.add_argument('command', nargs='*', default='')
parser.add_argument('--config', help='''Optional yaml file to handle command line arguments
(except positional args) as a config file. This config will override any other command line
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change the order of precedence here. The command line should be higher priority than the config file. It's closer to the user and has been explicitly typed out on the command line so should take precedence.

This will be really convenient to have standard configurations. And then you can override or extend for a specific need on the commandline reusing the saved config file that's closest.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I 100% agree. I spent a little while fiddling with it before submitting this PR but couldn't get it to work and I gave up. Do you know of a way to determine which args from argparse are explicitly passed? I've seen a few methods out there (namely creating a second namespaced parser to compare against), but none of them work as-is with the non-trivial argument options that rocker has so it requires some more digging.

One way I had initially thought of was to check for default values in the argument list, but if you explicitly want to override a config option with a default value, you won't be able to. If you don't have any immediate ideas, I'll find another way to make it work

arguments of the same name as the yaml keys (e.g. "--user-override-name" would have the key
"user_override_name" in the config file)''')
parser.add_argument('--noexecute', action='store_true', help='Deprecated')
parser.add_argument('--nocache', action='store_true')
parser.add_argument('--nocleanup', action='store_true', help='do not remove the docker container when stopped')
Expand All @@ -50,6 +55,12 @@ def main():
args = parser.parse_args()
args_dict = vars(args)

# Load config file if provided
if args.config:
with open(args.config, 'r') as f:
config = yaml.safe_load(f)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we should be doing some validation here.

And providing some visibility as to what's being loaded and from where as well. Think about how one would debug a typo in a config value or confirm that something is being selected.

args_dict.update(config)

if args.noexecute:
from .core import OPERATIONS_DRY_RUN
args_dict['mode'] = OPERATIONS_DRY_RUN
Expand Down
Loading