-
Notifications
You must be signed in to change notification settings - Fork 999
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
Use API link to download datasets #13681
Use API link to download datasets #13681
Conversation
Since the `history_id` is really only used for consistency with other history contents routes, we can make it optional and create a route alias for the datasets API
Do we have a download test? |
@router.get( | ||
"/api/datasets/{history_content_id}/display", | ||
summary="Displays (preview) or downloads dataset content.", | ||
response_class=StreamingResponse, | ||
) |
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.
Would the alias parameter work here ? https://github.com/galaxyproject/galaxy/pull/13648/files#diff-dd77d9205742e66941b40c5ab68c4205fcb4eee020a872d4eee6ea2d8ef9ac89R257
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.
It should work, but then this route will appear under histories
or the other one under datasets
because I can not set a different tag for the alias right?
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.
If the tag is not important, I'm happy to use the new alias parameter :)
We do have some tests for the |
Fixes #13679
Since the
history_id
in/api/histories/{history_id}/contents/{history_content_id}/display
is only really used in the path for consistency with other history contents endpoint, I made it optional, removed the parameter from the service and added a route alias for/api/datasets/{history_content_id}/display
.Now the download links in both legacy and new history use the API link.
How to test the changes?
License