Skip to content
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

Feature Request: Download a directory inside the output using cli #226

Merged
merged 3 commits into from
Nov 29, 2018

Conversation

ReDeiPirati
Copy link
Contributor

@ReDeiPirati ReDeiPirati commented Nov 20, 2018

Now it's possible to download a directory from a Job or dataset with the floyd [data] clone --directory <dir_name> ID

@ReDeiPirati ReDeiPirati requested review from narenst and houqp November 20, 2018 17:47

(e.g. foo/projects/bar/1/files, foo/projects/bar/1/output or foo/dataset/bar/1/

This will download the files that are returned at
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This will download the files that are returned at
This will download the files that are saved at

data_url = "{}/api/v1/resources/{}?content=true&download=true".format(floyd.floyd_host,
data_source.resource_id)
# Download a directory from Dataset or Files
else:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For readability, can you change the if condition and swap these cases?

if directory:
    # Download a directory from Dataset or files
    ...
else:
    ...

@@ -141,9 +143,17 @@ def print_data(data_sources):

@click.command()
@click.argument('id', nargs=1)
def clone(id):
@click.option('--directory', '-d',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would more obvious to call this option --path instead of directory.

@@ -141,9 +143,17 @@ def print_data(data_sources):

@click.command()
@click.argument('id', nargs=1)
def clone(id):
@click.option('--directory', '-d',
help='Download a directory from Job output or a dataset')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
help='Download a directory from Job output or a dataset')
help='Download files in a specific path from job output or a dataset')

- Download all files in a dataset or from Job output
- Download a directory from a dataset or from Job output

(e.g. foo/projects/bar/1/files, foo/projects/bar/1/output or foo/dataset/bar/1/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This documentation is a bit difficult to follow. Here is my recommendation:

- Download all files in a dataset or from a Job output

Eg: alice/projects/mnist/1/files, alice/projects/mnist/1/output or alice/dataset/mnist-data/1/

Using /output will download the files that are saved at the end of the job.

- Download a directory from a dataset or from Job output

Specify the path to a directory and download all its files and subdirectories.

Eg: --path models/checkpoint1

Copy link
Contributor

@narenst narenst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@houqp houqp merged commit 29b25be into master Nov 29, 2018
@houqp houqp deleted the out-dir branch November 29, 2018 19:31
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.

3 participants