-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
pypodman: Don't use $HOST and $USER variables for remote #1824
pypodman: Don't use $HOST and $USER variables for remote #1824
Conversation
8734e2c
to
4f14859
Compare
/ok-to-test |
It would be OK to use something specific, like $PYPODMAN_USER and $PYPODMAN_HOST/PORT I just hid all that complexity in the Currently you have to override $XDG_CONFIG_HOME, which also hard (a bad idea) to do globally: """Find and fill in any arguments not passed on command line."""
args.xdg_runtime_dir = os.environ.get('XDG_RUNTIME_DIR', '/tmp')
args.xdg_config_home = os.environ.get('XDG_CONFIG_HOME',
os.path.expanduser('~/.config'))
args.xdg_config_dirs = os.environ.get('XDG_CONFIG_DIRS', '/etc/xdg') Would be better to set $PYPODMAN_CONFIG_HOME, when you want to use a temporary config ? Anyway, this change allows me to not have to use I will do a separate patch for the variable. Already contributed a patch for |
I believe the variables should be prefixed with PODMAN_ not PYPODMAN. That would give us the flexibility to name the tool |
It would also give you the opportunity to use a different language :-) Maybe the same approach should be taken for the pypodman.conf ? |
Also, don't use $PORT. These are too generic. Make sure to read $LOGNAME _after_ the config. Prefix all the remote variables with PODMAN_ Signed-off-by: Anders F Björklund <[email protected]>
4f14859
to
4adc69f
Compare
@jwhonce : added it to all environment variables, for consistency. |
LGTM |
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
/lgtm |
LGTM |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Also, don't use $PORT. These are too generic.
Make sure to read $LOGNAME after the config.
Makes it hard to configure "root" in pypodman.conf
It will always use $USER, for the ssh login name ?