Skip to content

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

Merged
j23414 merged 1 commit intomasterfrom
drop-wnv-wa-tree
Mar 25, 2025
Merged

Migrate WA tree from WNV list to NW-PaGe#1135
j23414 merged 1 commit intomasterfrom
drop-wnv-wa-tree

Conversation

@j23414
Copy link
Contributor

@j23414 j23414 commented Mar 21, 2025

Description of proposed changes

Part of migration of WA WNV trees to NW-PaGe

Related issue(s)

Checklist

  • Checks pass
  • Check if changes affect the resource index JSON revision
  • Optional step: Add NW-PaGe to manifest [Edit: 2025-03-24] Noop, migrate to Nextstrain group so different system

@nextstrain-bot nextstrain-bot temporarily deployed to nextstrain-s-drop-wnv-w-o8gyt6 March 21, 2025 22:06 Inactive
@victorlin
Copy link
Member

Optional step: Add NW-PaGe to manifest

I assume migration to NW-PaGe is to either a Nextstrain group or community dataset. If that's the case, it doesn't make sense to add to data/manifest_core.json which is only for core datasets.

@j23414
Copy link
Contributor Author

j23414 commented Mar 24, 2025

Thanks for the catch! Agree, the migration afaik is to a Nextstrain group, not core, or community.

I'll update this PR as ready for review

@j23414 j23414 marked this pull request as ready for review March 24, 2025 17:25
@j23414 j23414 changed the title Draft: Migrate WA tree from WNV list to NW-PaGe Migrate WA tree from WNV list to NW-PaGe Mar 24, 2025
"WNV": {
"resolution": {
"all-lineages": "",
"wa": "",
Copy link
Member

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

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

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

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

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

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

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).

Copy link
Member

@victorlin victorlin left a comment

Choose a reason for hiding this comment

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

I assume this needs coordination with the workflow side of things, i.e. nextstrain/WNV#81, so I trust your judgement on when to merge this.

@j23414 j23414 merged commit 803e567 into master Mar 25, 2025
8 checks passed
@j23414 j23414 deleted the drop-wnv-wa-tree branch March 25, 2025 19:19
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