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.

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

Add Nextclade dataset version tracking file #390

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add Nextclade dataset version tracking file #390

Show file tree
Hide file tree
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
16 changes: 12 additions & 4 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,17 +135,21 @@ Whenever the underlying nextclade dataset (reference tree, QC rules) and/or next
In order to tell ingest to not use the cached `nextclade.tsv`/`aligned.fasta` and instead perform a full rerun, you need to add an (empty) touchfile to the s3 bucket:

```bash
aws s3 cp - s3://nextstrain-ncov-private/nextclade.tsv.zst.renew < /dev/null
aws s3 cp - s3://nextstrain-data/files/ncov/open/nextclade.tsv.zst.renew < /dev/null
for file in [ "nextclade.tsv.zst.renew", "version_sars-cov-2.txt" ]; do
aws s3 cp - s3://nextstrain-ncov-private/$file < /dev/null
aws s3 cp - s3://nextstrain-data/files/ncov/open/$file < /dev/null
done
Comment on lines +138 to +141
Copy link
Member

Choose a reason for hiding this comment

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

This is syntactically valid bash, but not what you intended. It'd produce four loop iterations where:

file="["
file="nextclade.tsv.zst.renew,"
file="version_sars-cov-2.txt"
file="]"

🙃

You want:

Suggested change
for file in [ "nextclade.tsv.zst.renew", "version_sars-cov-2.txt" ]; do
aws s3 cp - s3://nextstrain-ncov-private/$file < /dev/null
aws s3 cp - s3://nextstrain-data/files/ncov/open/$file < /dev/null
done
for file in nextclade.tsv.zst.renew version_sars-cov-2.txt; do
aws s3 cp - s3://nextstrain-ncov-private/$file < /dev/null
aws s3 cp - s3://nextstrain-data/files/ncov/open/$file < /dev/null
done

```

Ingest will automatically remove the touchfiles after it has completed the rerun.

To rerun Nextclade using the `sars-cov-2-21L` dataset - which is only necessary when the calculation of `immune_escape` and `ace2_binding` changes - you need to add an (empty) touchfile to the s3 bucket:

```bash
aws s3 cp - s3://nextstrain-ncov-private/nextclade_21L.tsv.zst.renew < /dev/null
aws s3 cp - s3://nextstrain-data/files/ncov/open/nextclade_21L.tsv.zst.renew < /dev/null
for file in [ "nextclade_21L.tsv.zst.renew", "version_sars-cov-2-21L.txt" ]; do
aws s3 cp - s3://nextstrain-ncov-private/$file < /dev/null
aws s3 cp - s3://nextstrain-data/files/ncov/open/$file < /dev/null
done
Comment on lines +149 to +152
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

```

## Required environment variables
Expand All @@ -157,3 +161,7 @@ aws s3 cp - s3://nextstrain-data/files/ncov/open/nextclade_21L.tsv.zst.renew < /
- `AWS_SECRET_ACCESS_KEY`
- `SLACK_TOKEN`
- `SLACK_CHANNELS`

## Unstable files produced by workflow

- `version_sars-cov-2.txt` and `version_sars-cov-2-21L.txt`: used to track the version of the nextclade dataset used to generate the `nextclade.tsv` and `nextclade_21L.tsv` files. Format: `timestamp dataset_version` (e.g. `2023-02-06T14:40:23Z 2023-02-01T12:00:00Z`) for each run since and including the last full run.
Copy link
Contributor

Choose a reason for hiding this comment

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

Am I understanding correctly that the first line of the versions file is the last full Nextclade run?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's what it should be in practice!

17 changes: 15 additions & 2 deletions workflow/snakemake_rules/nextclade.smk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,13 +138,26 @@ rule download_nextclade_executable:
"""

rule download_nextclade_dataset:
"""Download Nextclade dataset"""
"""
Download Nextclade dataset
Append the dataset version used for this run to the version file with timestamp of download time
"""
input: "nextclade"
output:
dataset = "data/nextclade_data/{dataset_name}.zip"
dataset = "data/nextclade_data/{dataset_name}.zip",
version = "data/nextclade_data/version_{dataset_name}.txt",
params:
dst_version_file=config["s3_dst"] + "/version_{dataset_name}.txt",
src_version_file=config["s3_src"] + "/version_{dataset_name}.txt",
shell:
"""
./bin/download-from-s3 {params.dst_version_file} {output.version} 0 || \
./bin/download-from-s3 {params.src_version_file} {output.version} 0 || \
touch {output.version}
Comment on lines +149 to +156
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be best to move this to a separate rule so that we can continue to keep the main pipeline untangled from the S3 interactions. I think this file can follow our current pattern of having a create_empty_* and a download_*_from_s3 rule so that this pipeline can run without S3 interactions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, not something I've ever done so wasn't quite aware this was a guarantee we make.

Copy link
Member

Choose a reason for hiding this comment

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

More of maintaining a separation of concerns than a guarantee, I think. (Although there are a few external users of ncov-ingest who might notice if that separation breaks down.)


./nextclade dataset get --name="{wildcards.dataset_name}" --output-zip={output.dataset} --verbose
printf %s "$(date --utc +%FT%TZ) " >> {output.version}
nextclade dataset list --name="{wildcards.dataset_name}" --json | jq -r '.[0].attributes.tag.value' >>{output.version}
Comment on lines 158 to +160
Copy link
Member

Choose a reason for hiding this comment

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

Mixing of ./nextclade vs. nextclade. Is that a mismatch?

Comment on lines +159 to +160
Copy link
Member

@tsibley tsibley Feb 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

There should be only one writer to this file, I think, but it'd still be more robust (and IMO clearer) to append the whole line at once rather than in two separate operations.

"""

GENES = "E,M,N,ORF1a,ORF1b,ORF3a,ORF6,ORF7a,ORF7b,ORF8,ORF9b,S"
Expand Down
26 changes: 25 additions & 1 deletion workflow/snakemake_rules/upload.smk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,29 @@ rule remove_rerun_touchfile:
touch {output}
"""

rule upload_dataset_version:
"""
Upload the Nextclade dataset version file
"""
input:
metadata_upload = f"data/{database}/metadata.tsv.zst.upload",
version_file = "data/nextclade_data/version_{dataset_name}.txt"
output:
touch("data/nextclade_data/version_{dataset_name}.upload")
params:
quiet = "" if send_notifications else "--quiet",
s3_bucket = config.get("s3_dst",""),
cloudfront_domain = config.get("cloudfront_domain", ""),
remote_filename = "version_{dataset_name}.txt",
shell:
"""
./bin/upload-to-s3 \
{params.quiet} \
{input.version_file:q} \
{params.s3_bucket:q}/{params.remote_filename:q} \
{params.cloudfront_domain} 2>&1 | tee {output}
"""

rule upload:
"""
Requests one touch file for each uploaded remote file
Expand All @@ -117,6 +140,7 @@ rule upload:
"nextclade.tsv.zst",
"nextclade_21L.tsv.zst",
]
]
],
dataset_version = [f"data/nextclade_data/version_{dataset_name}.upload" for dataset_name in ["sars-cov-2", "sars-cov-2-21L"]],
output:
touch(f"data/{database}/upload.done")