-
Notifications
You must be signed in to change notification settings - Fork 17
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
added functionality to latch test-data upload #194
Conversation
Added the option to specify destination folder to write test-data to
Related to this issue: #187 |
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.
apologies for the delay in getting this reviewed, i have some small changes, but otherwise looks good
|
||
from latch_cli.services.test_data.utils import _retrieve_creds | ||
from latch_cli.utils import account_id_from_token, retrieve_or_login | ||
|
||
BUCKET = "latch-public" | ||
|
||
|
||
def upload(src_path: str, dont_confirm_overwrite: bool = True) -> str: | ||
def upload(src_path: str, s3_path: str = None, dont_confirm_overwrite: bool = True) -> str: |
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.
def upload(src_path: str, s3_path: str = None, dont_confirm_overwrite: bool = True) -> str: | |
def upload(src_path: str, s3_path: Optional[str] = None, dont_confirm_overwrite: bool = True) -> str: |
using the Optional
type from typing
makes this more semantic
@@ -506,6 +506,11 @@ def test_data(ctx): | |||
|
|||
@test_data.command("upload") | |||
@click.argument("src_path", nargs=1, type=click.Path(exists=True)) | |||
@click.option( | |||
"--s3_path", |
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.
"--s3_path", | |
"--s3-path", |
kebab-case for cli options
@click.option( | ||
"--s3_path", | ||
default = None, | ||
type=str, |
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.
can you make this optional as well (see my other comment)
Added the option to specify a destination folder on s3 when uploading test-data.
For example:
latch test-data upload./matrix.mtx --s3_path test/matrix.mtx
will write tos3://latch-public/test-data/4751/test/matrix.mtx
.If you don't specify
--s3_path
, it will write to the same path as the local path. Default functionality remains the same.