-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
feat(#501): enables to copy file from cluster into local container #725
base: main
Are you sure you want to change the base?
Changes from 3 commits
9b6800f
a619730
b8e6e88
e60e174
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
from typing import Any, Dict, Iterable, List, Optional, Tuple, Union | ||
import click | ||
from click import ClickException | ||
from typing import NamedTuple | ||
|
||
|
||
def standard_error_handler(func): | ||
|
@@ -237,6 +238,42 @@ def parse_workload(ctx, param, workload: str) -> str: | |
return workload | ||
|
||
|
||
class FileFromArgument(NamedTuple): | ||
workload: str | ||
source: str | ||
destination: str = "/" | ||
|
||
|
||
def parse_file_from(ctx, param, file_from_arguments) -> Tuple[FileFromArgument]: | ||
if not file_from_arguments: | ||
return None | ||
|
||
MSG = ( | ||
"Invalid argument format. Please provide the file " | ||
"in the format 'type/name:source:destination' or " | ||
"'type/name/container-name:source:destination'." | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we make destination optional? (I would suspect in most of the cases users will want the file to land in the same place it was copied from.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I agree. Good point. |
||
) | ||
|
||
file_from = [] | ||
for file_from_argument in file_from_arguments: | ||
split_argument = file_from_argument.split(":") | ||
if len(split_argument) != 3: | ||
raise ValueError(MSG) | ||
|
||
workload, source, destination = split_argument | ||
if "/" not in workload: | ||
raise ValueError(MSG) | ||
|
||
file_from_argument_parsed = FileFromArgument( | ||
workload=workload, | ||
source=source, | ||
destination=destination, | ||
) | ||
file_from.append(file_from_argument_parsed) | ||
|
||
return tuple(file_from) | ||
|
||
|
||
def check_connection_name(ctx, param, selected: Optional[str] = None) -> str: | ||
from gefyra import api | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all thank you veeeeery much for this PR! This will reduce the number of things which otherwise need to be done in some wrapper script!
pls allow me to add some general questions/comments:
If I can help with more details, I’d more than happy to do so. Please feel free to reach out to me on email or phone any time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree to most of the mentioned points, however, cannot answer all, especially the first point.
@SteinRobert brought up the same point. I will see how this can be implemented. We thought that a limit of around 50MB max might be good. I personally only transfer files of some KB, thus I don't have any feeling for a reasonable limit for other use cases.
I guess. I will check it out. This would also reduce the count of connections etc and might play nice together with the transfer limit.
Yes.