Skip to content

Migrate WA tree from WNV list to NW-PaGe #1135

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.

Sign up for GitHub

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

Merged
j23414 merged 1 commit into from
Mar 25, 2025
Merged

Migrate WA tree from WNV list to NW-PaGe #1135

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion data/manifest_core.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -779,7 +779,6 @@
"WNV": {
"resolution": {
"all-lineages": "",
"wa": "",
Copy link
Member

@victorlin victorlin Mar 24, 2025

Choose a reason for hiding this comment

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

This renders the URL https://nextstrain.org/WNV/wa invalid. Is the new NW-PaGe dataset ready? It should be possible to encode a redirect in src/redirects.js from /WNV/wa to /groups/… if desired. Note that it would be a redirect for the direct URL, not an entry in the dataset selector which is based on manifest.

Copy link
Member

@victorlin victorlin Mar 24, 2025

Choose a reason for hiding this comment

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

Wait, actually it will still work, but we don't want that as it'll become out of date if it's not automatically updated by the workflow: https://nextstrain-s-drop-wnv-w-o8gyt6.herokuapp.com/WNV/wa

To properly remove /WNV/wa, it needs to be removed from S3.

Copy link
Contributor Author

@j23414 j23414 Mar 24, 2025

Choose a reason for hiding this comment

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

Since we just migrated last week and I know some people are busy this week, I'd estimate that the NW-PaGe dataset might not be ready until April 4th or later.

Good to know about the redirects! I can ping NW-PaGe next week for an update/if they would like the redirect option. In the meantime, I don't think we need to maintain a valid URL https://nextstrain.org/WNV/wa as it seems like NW-PaGe would like to manage the WA view themselves

Copy link
Contributor Author

@j23414 j23414 Mar 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

To properly remove /WNV/wa, it needs to be removed from S3.

I'm happy to nextstrain remote rm from S3

I'll probably do a nextstrain remote ls check for WNV Auspice datasets. I probably need to remove some old non-updating "global". I'll leave NA since that's tied to a narrative

Copy link
Member

@victorlin victorlin Mar 24, 2025

Choose a reason for hiding this comment

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

The redirect would be more for ourselves so we don't break our own URL. I see that /WNV/wa was only recently added in ff1e6d2 so maybe it's fine to break. Another option is to redirect to an existing core dataset, i.e. /WNV/wa → /WNV/all-lineages. I don't have any opinions here, just want to list out some options.

Copy link
Contributor Author

@j23414 j23414 Mar 24, 2025

Choose a reason for hiding this comment

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

That's fair! Thanks for listing the other options, and this thread does help clarify my thinking on this and similar PRs 😄 No worries!

I lean toward not supporting /WNV/wa -> /WNV/all-lineages redirects as /WNV/wa was only recently added, and was fated to be dropped anyway. And as a user, I'd be confused on why "WNV/wa (state)" was sending me to WNV/all-lineages (global)

Copy link
Member

@victorlin victorlin Mar 25, 2025

Choose a reason for hiding this comment

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

I'm happy to nextstrain remote rm from S3

👍

I probably need to remove some old non-updating "global".

You could also redirect /WNV/global to /WNV/all-lineages (in addition to removing the global JSON from S3).

"lineage-1A": "",
"lineage-2": "",
"NA": "",
Expand Down