Declare connection plugin variables and use them with get_option #31
Conversation
|
testing it right now with debops.lxc |
dmp1ce
force-pushed
the
issue-30-declare-variables-add-lxc_host-var
branch
from
90227e3 to
b60d14f
Compare
lxc_ssh.py
Outdated
| self._add_args( | ||
| b_command, ( | ||
| b"-o", b"KbdInteractiveAuthentication=no", | ||
| b"-o", b"PreferredAuthentications=publickey,gssapi-with-mic,gssapi-keyex,hostbased", | ||
| b"-o", b"PreferredAuthentications=gssapi-with-mic,gssapi-keyex,hostbased,publickey", |
There was a problem hiding this comment.
Minor question: why do you turn the authentication methods around?
In your PR "publickey" is now the last methog, and all the other ones need to be checked first, that is time-consuming.
Is there a logical explanation for this?
There was a problem hiding this comment.
Yeah, I blindly copied over the settings from ssh.py. I figured ssh.py had it in that order for a reason. Most likely this line change is not needed for the #30 fix.
There was a problem hiding this comment.
Ok, I think when you remove this from the PR I can merge it.
There was a problem hiding this comment.
My latest force push removes this change. 4e569a7
|
@dmp1ce Thanks for verifying. I have one last code question left, which is more a performance issue. See the inline comment please. |
|
Oh, and there is another PR merged in the meantime, that might require another rebase of your PR. |
dmp1ce
force-pushed
the
issue-30-declare-variables-add-lxc_host-var
branch
2 times, most recently
from
e6f8983 to
774c2d6
Compare
- Declare all SSH variables from ssh.py - Use self.get_option where required - Update the _build_command from latest ssh.py - Declare lxc_host variable for declaring the container name to use
dmp1ce
force-pushed
the
issue-30-declare-variables-add-lxc_host-var
branch
from
774c2d6 to
4e569a7
Compare
|
Thanks for this PR! Really appreciated. |
Fixes #30
The
lxc_hostneeded to be created because the new_build_commandfunction messes with the ssh args I think. I couldn't get the script to work without adding thelxc_hostvariable which I think makes more sense for the plugin anyway.