Adding SCP support #2642
Adding SCP support #2642
Conversation
.travis.yml
Outdated
| dist: precise | ||
| language: python | ||
| sudo: false | ||
| sudo: true |
| <td width="30px"> </td> | ||
| <td> | ||
| <button type="button" class="btn btn-light" onclick="$('input[name*=\'files_to_erase\']').removeAttr('checked');">Select None</button> | ||
| <button type="button" class="btn btn-light" onclick="$('input[name*=\'files_to_erase\']').removeAttr('checked');">Unselect All</button> |
There was a problem hiding this comment.
This means that you have to do: git pull upstream dev
|
|
||
| def private_task(job_id): | ||
| """Complets a Qiita private task | ||
| """Completes a Qiita private task |
Codecov Report
@@ Coverage Diff @@
## dev #2642 +/- ##
=======================================
Coverage 94.34% 94.34%
=======================================
Files 164 164
Lines 19416 19416
=======================================
Hits 18318 18318
Misses 1098 1098Continue to review full report at Codecov.
|
|
@ElDeveloper, could you take a look? Thanks! |
There was a problem hiding this comment.
Overall looks good, but there's a few things that we've (me and @charles-cowart) noted.
.gitignore
Outdated
| # webdis log | ||
| webdis.log | ||
|
|
||
| # test keys should generated in travis |
There was a problem hiding this comment.
should generated -> should be generated
qiita_ware/commands.py
Outdated
| Parameters | ||
| ---------- | ||
| URL : urlparse object |
| port = p_url.port | ||
| username = p_url.username | ||
|
|
||
| if scheme == 'scp' or scheme == 'sftp': |
There was a problem hiding this comment.
This needs an alternative branch that raises an appropriate exception if the scheme is not supported.
qiita_ware/commands.py
Outdated
| if scheme == 'scp' or scheme == 'sftp': | ||
|
|
||
| # if port not specified, use default 22 as port | ||
| if not port: |
There was a problem hiding this comment.
This should probably be more explicit about the port not being specified. It should be if port is None:
qiita_ware/commands.py
Outdated
| if not port: | ||
| port = 22 | ||
|
|
||
| # step 1: both schemes requires an SSH connection |
| url = job.parameters.values['url'] | ||
| private_key = job.parameters.values['private_key'] | ||
| try: | ||
| list_remote(url, private_key) |
There was a problem hiding this comment.
What is happening to the output of this function, it seems like it is not being utilized anywhere and that this job does nothing(?)
| read_file_list = list_remote('scp://localhost:'+self.remote_dir_path, | ||
| self.test_ssh_key) | ||
| remote_filename_list = [basename(f) for f in | ||
| glob(join(self.remote_dir_path, '*'))] |
There was a problem hiding this comment.
This wildcard doesn't seem consistent with the checks, since we only allow the extensions that are described in the qiita config file. I would suggest that the expected list be hardcoded instead.
| self.assertEqual(remote_filename_list, local_filename_list) | ||
|
|
||
| def test_download_sftp(self): | ||
| """Tests remote file listing using private key and sftp""" |
There was a problem hiding this comment.
should be Tests remote file downloading ...
| read_file_list = list_remote('sftp://localhost:'+self.remote_dir_path, | ||
| self.test_ssh_key) | ||
| remote_filename_list = [basename(f) for f in | ||
| glob(join(self.remote_dir_path, '*'))] |
| download_remote('scp://localhost:'+self.remote_dir_path, | ||
| self.test_ssh_key, self.temp_local_dir) | ||
| remote_filename_list = [basename(f) for f in | ||
| glob(join(self.remote_dir_path, '*'))] |
|
This PR has been superseded by #2660 |
4 participants
Note that it doesn't support connecting with passwords and instead requires ssh keys.