Skip to content

Conversation

@pkittenis
Copy link
Member

Added support for recursive copying to copy_file. Now, when a directory path is supplied to local_file, the method recursively calls itself on every file and directory within that directory, and attempts to copy them to the directory path supplied to remote_file.

New one to fix bad rebase showing incorrect differences.

Kincaid Savoie added 24 commits October 24, 2015 16:04
… into sftp_recursive

Conflicts:
	pssh/ssh_client.py
… into sftp_recursive

Conflicts:
	pssh/ssh_client.py

Merged in new mkdir function.
@pkittenis
Copy link
Member Author

Could we get a test for the failing behaviour, ie what happens when copy_file is called with local_file being a directory but with recurse=False.

Can copy/paste the test you have and not supply recurse parameter.

copy_file probably just needs an elif not recurse and some error logging to tell the user they are calling the function with a directory path but they need to supply recurse=True for it to do anything.

@Caid11
Copy link
Contributor

Caid11 commented Oct 30, 2015

Sure, I'll add a test that just doesn't supply the recurse parameter. Should I throw some kind of exception when the user tries to copy a directory without setting recurse to true?

@Caid11
Copy link
Contributor

Caid11 commented Oct 30, 2015

I decided to have it throw an exception when a directory is supplied to copy_file but recurse isn't set; I think that's the most straightforward way to let users know what they're doing wrong. I also added a test making sure that happens.

@pkittenis
Copy link
Member Author

Thanks for the test @Caid11 - looks great.

One last thing, can we get the docstring for copy_file updated for the new exception that is raised?

Can see the syntax at https://github.com/pkittenis/parallel-ssh/blob/master/pssh/ssh_client.py#L143

@Caid11
Copy link
Contributor

Caid11 commented Nov 3, 2015

Sure thing.

pkittenis added a commit that referenced this pull request Nov 4, 2015
Allow recursive copy via SFTP on `copy_file` function - resolves #21
@pkittenis pkittenis merged commit bc81728 into ParallelSSH:master Nov 4, 2015
@pkittenis
Copy link
Member Author

Thanks for doing this @Caid11 - much appreciated.

Forgot something - pssh_client.py needs to be updated to pass on the recurse parameter to copy_file and where copy_file on SSHClient is called so the recurse functionality works over the ParallelSSHClient.

Also same docstring change for the new parameter and exception.

Can do this next time I'm in the code, very small change, or submit a new PR for it if you want (I don't think it's possible to add new commits to a merged PR).

NB - SSHClient is a single SSH client against a particular host. Many SSHClient objects are used by ParallelSSHClient to do things over multiple hosts. ParallelSSHClient calls SSHClient functions directly, via gevent pool.

New functionality in SSHClient needs adding to ParallelSSHClient to be used there.

@Caid11
Copy link
Contributor

Caid11 commented Nov 9, 2015

Sorry, I didn't notice your comment until today. I'll open a PR to fix it after I'm done with the current one. If you happen to fix it before I do, that's fine too.

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

Successfully merging this pull request may close these issues.

2 participants