Qhughes/adding scp support - #8
Conversation
ohinds
left a comment
ohinds
left a comment
There was a problem hiding this comment.
This looks really good! I know some of these are picky, sorry about that :P Very close though, I think we can get this merged soon.
| parser.add_argument( "instance", type=lambda name: query("Instance", "instance", name),) | ||
| parser.add_argument('connection_args', nargs=argparse.REMAINDER) | ||
|
|
||
| def args_instance_scp(parser): |
There was a problem hiding this comment.
I think put might be better here, as it's consistent with the get below.
There was a problem hiding this comment.
this is left over from the old ass ftp terminology. get and put.
| instance.send_scp(source, destination, scp_args) | ||
|
|
||
| @f0cal.entrypoint(["farm", "instance", "get"], args=args_instance_scp) | ||
| def instance_get(parser, core, instance, source, scp_args, *args, **kwargs): |
There was a problem hiding this comment.
I like the get terminology, but it seems like the args here should be args_instance_get?
There was a problem hiding this comment.
I actually removed args_instance_get, both get and put use args_instance_scp
| @f0cal.entrypoint(["farm", "instance", "send"], args=args_instance_scp) | ||
| def instance_send(parser, core, instance, source, scp_args, *args, **kwargs): | ||
| if '-destination' in scp_args: | ||
| i = scp_args.index('-destination') |
There was a problem hiding this comment.
I think we need two dashes, like --destination
| destination = scp_args.pop(i + 1) | ||
| del scp_args[i] | ||
| else: | ||
| destination = "~/" |
There was a problem hiding this comment.
Because it's not a string that will be printed for the user, these should be single quotes
| def instance_send(parser, core, instance, source, scp_args, *args, **kwargs): | ||
| if '-destination' in scp_args: | ||
| i = scp_args.index('-destination') | ||
| if len(scp_args) == i+1: |
| if not _fn: | ||
| print(f'{connection_type} connections not supported') | ||
| exit(1) | ||
| print((connection_args)) |
| def send_scp(self, source, destination, send_args): | ||
| scp_bin = '/usr/bin/scp' | ||
| send_args = self._format_send_args(source, destination, send_args) | ||
| print('*'*80) |
There was a problem hiding this comment.
Need a space on either side of the * here.
ohinds
left a comment
ohinds
left a comment
There was a problem hiding this comment.
This looks great! Just one more suggestion.
| print('Destination directory must follow -destination flag, or remove the -destination flag to scp into the home directory.') | ||
| i = scp_args.index('--destination') | ||
| if len(scp_args) == i + 1: | ||
| print('Destination directory must follow --destination flag, or remove the -destination flag to scp into the home directory.') |
There was a problem hiding this comment.
Here update the second -destination to have two dashes, also?
ohinds
left a comment
ohinds
left a comment
There was a problem hiding this comment.
Looks great! Let's do a walkthrough the next time you are at standup.
… Qhughes/adding-scp-support
2 participants
Client now supports copying files via scp to and from instance