-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add support for Data Explorer functionality (list, add) #381
Comments
Presumably |
For me it's a -1. Why bloating the CLI with this? |
You could argue that it's not worth having a CLI at all, there's a perfectly good API! Having it in the CLI makes it faster and easier to work with datasets from the terminal. It improves developer / user experience. |
Also the technical reason for having it in the CLI for downloading files: Presigned URLs expire after a short-ish window (@swampie thought it was 1 hour). If downloading a large dataset, the download could easily run for many hours. A generated bash script would therefore fail, however the CLI could request the presigned URLs one at a time in series, meaning that they're always fresh and continue to work. |
Adding a usecase to download/list a dataset, with a flag to download/list the files inside the dataset csv/tsv/table. For example:
This downloads the dataset table (csv/tsv) plus the files. In this way the user only has to be concerned with passing around the dataset object, and they can download/list the files at any time. Think a dataset can also be an output so it becomes a packaging mechanism. Note: Today, the auth to access/download/list files in a dataset is not guaranteed as users can create whatever s3:// paths they want in a csv. This issue also exists when launching a pipeline. |
Fair enough |
for upload and download why using the seqera cli when you can use the standard cloud tooling? |
|
Considering the ongoing work to extend the Data Explorer availability to personal workspaces, these new CLI capability should be implemented for those as well. |
I agree with Paolo that the complexity is not justified for the time being: open to discuss |
Adding a very key point being lost here. Our end users shouldn't need cloud console or cloud provider CLI access. They likely don't have cloud credentials. This is the point of having different roles with WS admins adding credentials and CEs. End users want to upload data, run pipelines, and download results. |
I agree 💯 that CLI should have first-class support. However, my understanding is that the feature highlighted here does not come for free, it may require some specific endpoints. |
Updated original request to match the Data Explorer |
TBD - pagination is always returned by the API, need to account for this in the CLI commands. |
I feel a bit weird about naming this subcommand The sub-title where you can list your "data links" says, "Browse remote data repositories and data for use in Seqera Cloud," with no reference to this "data link" concept. Overall, this "data link" concept is misleading. I'd call it "data source", and then the command line can be But because naming is difficult and what sounds good to me may sound terrible to others, I suggest reviewing this naming before hardcoding it into the command line interface. Or at least, if "data link" is chosen as the best way of naming it, the web UI should be consistent and call that section "data links" instead of "data explorer" with explicit references to the "data link" concept when you add a new one. |
@jordeu Thanks for the feedback! The Data Explorer API endpoint is called |
@robnewman we are missing here the method to list content |
@weronikasosnowskaseqera Please add. I wasn't necessarily comprehensive - just that the functionality needs to exist and reflect the API functionality. |
This issue has been unlinked from a Canny post: Add datasets directly from s3 / data explorer to the platform 😢 |
This is now done except for the |
|
Add a new command:
that interacts with the new Data Explorer
data-links
API endpoint in the Seqera Platform. Some suggested functionality (need to add all the auth syntactic sugar):Tasks
The text was updated successfully, but these errors were encountered: