Allow to pass encoding when creating shell

[beenje]

The encoding parameter on run/spawn is nice, but it's a bit tedious to pass it to every run command.
It'd be nice to be able to pass it when creating the shell.

Something like that:

class SshShell(object):
    def __init__(self,
            hostname,
            username=None,
            password=None,
            port=None,
            private_key_file=None,
            connect_timeout=None,
            missing_host_key=None,
            shell_type=None,
            look_for_private_keys=True,
            load_system_host_keys=True,
            sock=None,
            encoding=None):

...

        self._encoding = encoding

    def spawn(self, command, *args, **kwargs):
        encoding = kwargs.pop("encoding", self._encoding)

We could then run:

import spur

shell = spur.SshShell(hostname="localhost", username="bob", password="password1", encoding="utf-8")

with shell:
    result = shell.run(["ls"])
    result = shell.run(["some_cmd"])

The above solution still allows to overwrite the encoding in the run command.

If you think it's a good idea, I can make a proper pull request.

[mwilliamson]

Thanks for the suggestion. I'm not sure there's anything particular special about the encoding argument -- you could equally argue that you should be able to set, say, a default cwd in the constructor. What about something like:

with spur.SshShell(...) as shell:
    run = partial(shell.run, encoding="utf-8")
    run(["ls"])

? Admittedly, this doesn't work so well if you're using both run and spawn.

[beenje]

My example wasn't the best. I don't really call many run commands in the same shell context, but I create the same shell to run commands in different functions.
Here is something close to what I did:

class MyShell(spur.SshShell):
    """Subclass spur.SshShell to force encoding on run/spawn"""

    def __init__(self, *args, **kwargs):
        self.encoding = kwargs.pop('encoding', None)
        super().__init__(*args, **kwargs)

    def spawn(self, *args, **kwargs):
        encoding = kwargs.pop('encoding', self.encoding)
        return super().spawn(*args, encoding=encoding, **kwargs)


def get_shell():
    return MyShell(hostname=current_app.config[REMOTE_NODE'],
                   missing_host_key=spur.ssh.MissingHostKey.accept,
                   encoding='utf-8', **current_app.config['SSH_SHELL'])

I can then use get_shell in different functions without having to specify the encoding when calling run/spawn.

I do think the encoding argument is a bit more special because it's quite common to want strings instead of raw bytes in stdout and stderr.

But it's only a suggestion. If you think this is a too specific use case, I don't mind subclassing SshShell. It doesn't take much code.

[mwilliamson]

I think for simplicity, it's better to just have the encoding argument in one place. Since you're happy subclassing shell, I'll close this issue, but I'll happily reconsider if other people think something similar would be useful for them.