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

Update lineage file to color samples on the mpox tree #1703

Merged
merged 2 commits into from
Aug 21, 2024
Merged

Conversation

danrlu
Copy link
Collaborator

@danrlu danrlu commented Aug 21, 2024

Summary:

Bring in the latest lineage info to color samples on the tree. See discussion here

Checklist:

  • I merged latest <base branch>
  • I manually verified the change
  • I added labels to my PR
  • I tested in multiple browsers
  • I added relevant unit tests
  • I have notified others of changes they need to make locally (migrations, jobs, package updates, etc)

@danrlu danrlu added the ez-pz Quick and easy to review label Aug 21, 2024
Copy link
Contributor

@vincent-czi vincent-czi left a comment

Choose a reason for hiding this comment

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

This seems like a reasonable extension of what's already happening!

Personally, I'm a bit confused how it's operating: I thought that the Dockerfile.nextstrain produced the docker image that the run_nextstrain_mpx.sh script ran inside of. So I'm surprised that we can run the chown during image build on files that we haven't downloaded yet? But we were already doing it for exclude_accessions_mpxv.txt and I assume that was fine then, so this seems like it would be fine as well? Perhaps the base nextstrain docker image already has those files present, so we can chown them during build, but then we download and swap-in-place so the ownership is now for the same file, but totally different content in the file? Dunno.

@danrlu
Copy link
Collaborator Author

danrlu commented Aug 21, 2024

Oh? I thought the files are downloaded into the image when the Docker image was built? Let me push it to staging and we can see whether it works as expected.

@danrlu danrlu merged commit a16a479 into trunk Aug 21, 2024
12 checks passed
@danrlu danrlu deleted the danrlu-patch-2 branch August 21, 2024 16:57
thanhleviet pushed a commit to theiagenghi/theiagenepi that referenced this pull request Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ez-pz Quick and easy to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants