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

Supporting downloading datasets not in manifest #345

Merged
merged 4 commits into from
Jan 18, 2024

Conversation

tsibley
Copy link
Member

@tsibley tsibley commented Jan 18, 2024

See commit messages for details.

(Based upon #333 for less in-flight conflict.)

Checklist

  • Checks pass

…e adapters

Moves them closer to the actual Charon API request/response and makes
the Resource/Dataset/Narrative model classes no longer tied to Charon's
response format.  The latter will be handy when manually instantiating
them in another location outside of the current places in _ls(), but
also make it easier when/if we move to another resource listing API.
… aren't in the manifest

This allows downloading of datasets like

    https://nextstrain.org/enterovirus/d68/vp1/2020-01-23
    https://nextstrain.org/nextclade/sars-cov-2/21L

and others, as reasonably expected.¹  It also will, with one more minor
tweak to follow, allow downloading of past snapshots of resources (e.g.
/zika@2023-01-01).

Switches from an assert on expected media type to a conditional
UserError, supported by the new Resource.__str__() method, since for
single resource downloads we no longer have the assurance of knowing it
exists already.

¹ <https://bedfordlab.slack.com/archives/C01LCTT7JNN/p1667970567194279>
…esources

Using the same @YYYY-MM-DD suffix syntax as on the web.  Support for
this server-side is recently landed.¹

¹ <nextstrain/nextstrain.org#719>
@tsibley tsibley requested a review from a team January 18, 2024 00:06
Copy link
Member

@jameshadfield jameshadfield left a comment

Choose a reason for hiding this comment

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

Functionality seems good by inspection

Base automatically changed from trs/remotes to master January 18, 2024 20:10
@tsibley tsibley merged commit d57031d into master Jan 18, 2024
36 checks passed
@tsibley tsibley deleted the trs/remote/download/not-in-manifest branch January 18, 2024 20:10
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.

2 participants