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.

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

RemoteCommands and nohup #261

Closed
magixx opened this issue Feb 10, 2016 · 4 comments
Closed

RemoteCommands and nohup #261

magixx opened this issue Feb 10, 2016 · 4 comments

Comments

@magixx
Copy link

magixx commented Feb 10, 2016

As far as I can tell attempting to run a command remotely via nohup is broken.
ex:
plumbum/machines/remote.py

    def nohup(self, cwd='.', stdout='nohup.out', stderr=None, append=True):
        """Runs a command detached."""
        return machine.nohup(self, cwd, stdout, stderr, append)

The referenced machine in nohup would need to be self.machine. However even with that change there seems to be the issue that SshMachine.nohup takes in completly different parameters and seems to getting deprecated. ex:
plumbum/machines/ssh_machine.py

    def nohup(self, command):
        """
        Runs the given command using ``nohup`` and redirects std handles,
        allowing the command to run "detached" from its controlling TTY or parent.
        Does not return anything. Depreciated (use command.nohup or daemonic_popen).
        """
        warnings.warn("Use .nohup on the command or use daemonic_popen)", DeprecationWarning)
        self.daemonic_popen(command, cwd='.', stdout=None, stderr=None, append=False)

Remote nohup should then perhaps be calling daemonic_popen of machine with proper parameters.

@henryiii
Copy link
Collaborator

So does the following new version work? It should, from a cursory glance:

    def nohup(self, cwd='.', stdout='nohup.out', stderr=None, append=True):
        """Runs a command detached."""
        return self.machine.daemonic_popen(self, cwd, stdout, stderr, append)

Since there is no way the current version is right, I pushed this change, can you test it?

@henryiii
Copy link
Collaborator

Assuming fixed.

@mbarkhau
Copy link

Doesn't the method commands.base.BaseCommand.nohup suffer from the same issue.

https://github.com/tomerfiliba/plumbum/blob/master/plumbum/commands/base.py#L139

@mbarkhau
Copy link

I'll just invoke the daemonic_popen myself for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants