-
Notifications
You must be signed in to change notification settings - Fork 9
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
feat(dag): add data transfer task group for release process #528
Changes from all commits
c2e3e91
9b64cf8
9ee712a
6e62c4b
fda650b
4dcf542
f12d4ea
088895a
db73c23
6fa81a6
cd3324a
cae2e40
d4f2792
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,10 @@ | ||
# Release specific configuration: | ||
release_version: "24.01" | ||
release_version: "24.03" | ||
dev_version: XX.XX | ||
release_folder: gs://genetics_etl_python_playground/releases/${datasets.release_version} | ||
|
||
inputs: gs://genetics_etl_python_playground/input | ||
static_assets: gs://genetics_etl_python_playground/static_assetss | ||
static_assets: gs://genetics_etl_python_playground/static_assets | ||
outputs: gs://genetics_etl_python_playground/output/python_etl/parquet/${datasets.dev_version} | ||
|
||
## Datasets: | ||
|
@@ -36,9 +36,9 @@ anderson: ${datasets.static_assets}/andersson2014/enhancer_tss_associations.bed | |
javierre: ${datasets.static_assets}/javierre_2016_preprocessed | ||
jung: ${datasets.static_assets}/jung2019_pchic_tableS3.csv | ||
thurman: ${datasets.static_assets}/thurman2012/genomewideCorrs_above0.7_promoterPlusMinus500kb_withGeneNames_32celltypeCategories.bed8.gz | ||
target_index: ${datasets.release_folder}/targets # OTP 23.12 data | ||
target_index: ${datasets.static_assets}/targets # OTP 23.12 data | ||
gene_interactions: ${datasets.static_assets}/interaction # OTP 23.12 data | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense to just point to our OTP release bucket? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might makes sense, however the service account can't read that bucket. And I don't see a reason to extend its permission to other buckets. In a way, while the platform and the genetics etls are separated, this solution is fine. |
||
|
||
gene_interactions: ${datasets.release_folder}/interaction # OTP 23.12 data | ||
finngen_finemapping_results_path: ${datasets.inputs}/Finngen_susie_finemapping_r10/full | ||
finngen_finemapping_summaries_path: ${datasets.inputs}/Finngen_susie_finemapping_r10/Finngen_susie_credset_summary_r10.tsv | ||
|
||
|
Large diffs are not rendered by default.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,160 @@ | ||
"""Test DAG to prototype data transfer.""" | ||
|
||
from __future__ import annotations | ||
|
||
from pathlib import Path | ||
|
||
import common_airflow as common | ||
from airflow.models.dag import DAG | ||
from airflow.operators.python import ShortCircuitOperator | ||
from airflow.providers.google.cloud.transfers.gcs_to_gcs import GCSToGCSOperator | ||
from airflow.utils.task_group import TaskGroup | ||
|
||
CLUSTER_NAME = "otg-etl" | ||
SOURCE_CONFIG_FILE_PATH = Path(__file__).parent / "configs" / "dag.yaml" | ||
|
||
# Release specific variables: | ||
RELEASE_VERSION = "24.03" | ||
RELEASE_BUCKET_NAME = "genetics_etl_python_playground" | ||
|
||
# Datasource paths: | ||
GWAS_CATALOG_BUCKET_NAME = "gwas_catalog_data" | ||
EQTL_BUCKET_NAME = "eqtl_catalogue_data" | ||
FINNGEN_BUCKET_NAME = "finngen_data" | ||
FINNGEN_RELEASE = "r10" | ||
|
||
# Files to move: | ||
DATA_TO_MOVE = { | ||
# GWAS Catalog summary study index: | ||
"gwas_catalog_study_index": { | ||
"source_bucket": GWAS_CATALOG_BUCKET_NAME, | ||
"source_object": "study_index", | ||
"destination_bucket": RELEASE_BUCKET_NAME, | ||
"destination_object": f"releases/{RELEASE_VERSION}/study_index/gwas_catalog", | ||
}, | ||
# PICS credible sets from GWAS Catalog curated associations: | ||
"gwas_catalog_curated_credible_set": { | ||
"source_bucket": GWAS_CATALOG_BUCKET_NAME, | ||
"source_object": "credible_set_datasets/gwas_catalog_curated", | ||
"destination_bucket": RELEASE_BUCKET_NAME, | ||
"destination_object": f"releases/{RELEASE_VERSION}/credible_set/gwas_catalog_pics_from_curation", | ||
}, | ||
# PICS credible sets from GWAS Catalog summary statistics: | ||
"gwas_catalog_sumstats_credible_set": { | ||
"source_bucket": GWAS_CATALOG_BUCKET_NAME, | ||
"source_object": "credible_set_datasets/gwas_catalog_summary_stats", | ||
"destination_bucket": RELEASE_BUCKET_NAME, | ||
"destination_object": f"releases/{RELEASE_VERSION}/credible_set/gwas_catalog_pics_from_summary_statistics", | ||
}, | ||
# GWAS Catalog manifest files: | ||
"gwas_catalog_manifests": { | ||
"source_bucket": GWAS_CATALOG_BUCKET_NAME, | ||
"source_object": "manifests", | ||
"destination_bucket": RELEASE_BUCKET_NAME, | ||
"destination_object": f"releases/{RELEASE_VERSION}/manifests", | ||
}, | ||
# eQTL Catalog study index: | ||
"eqtl_catalogue_study_index": { | ||
"source_bucket": EQTL_BUCKET_NAME, | ||
"source_object": "study_index", | ||
"destination_bucket": RELEASE_BUCKET_NAME, | ||
"destination_object": f"releases/{RELEASE_VERSION}/study_index/eqtl_catalogue", | ||
}, | ||
# eQTL Catalog SuSiE credible sets: | ||
"eqtl_catalogue_susie_credible_set": { | ||
"source_bucket": EQTL_BUCKET_NAME, | ||
"source_object": "credible_set_datasets/susie", | ||
"destination_bucket": RELEASE_BUCKET_NAME, | ||
"destination_object": f"releases/{RELEASE_VERSION}/credible_set/eqtl_catalogue_susie", | ||
}, | ||
# Finngen study index: | ||
"finngen_study_index": { | ||
"source_bucket": FINNGEN_BUCKET_NAME, | ||
"source_object": f"{FINNGEN_RELEASE}/study_index", | ||
"destination_bucket": RELEASE_BUCKET_NAME, | ||
"destination_object": f"releases/{RELEASE_VERSION}/study_index/finngen", | ||
}, | ||
# Finngen summary statistics: | ||
"finngen_PICS_credible_set": { | ||
"source_bucket": FINNGEN_BUCKET_NAME, | ||
"source_object": f"{FINNGEN_RELEASE}/credible_set_datasets/finngen_pics", | ||
"destination_bucket": RELEASE_BUCKET_NAME, | ||
"destination_object": f"releases/{RELEASE_VERSION}/credible_set/finngen_pics", | ||
}, | ||
# Finngen SuSiE credible sets: | ||
"finngen_susie_credible_set": { | ||
"source_bucket": FINNGEN_BUCKET_NAME, | ||
"source_object": f"{FINNGEN_RELEASE}/credible_set_datasets/finngen_susie_processed", | ||
"destination_bucket": RELEASE_BUCKET_NAME, | ||
"destination_object": f"releases/{RELEASE_VERSION}/credible_set/finngen_susie", | ||
}, | ||
# L2G gold standard: | ||
"gold_standard": { | ||
"source_bucket": "genetics_etl_python_playground", | ||
"source_object": "input/l2g/gold_standard/curation.json", | ||
"destination_bucket": RELEASE_BUCKET_NAME, | ||
"destination_object": f"releases/{RELEASE_VERSION}/locus_to_gene_gold_standard.json", | ||
}, | ||
} | ||
|
||
|
||
# This operator meant to fail the DAG if the release folder exists: | ||
ensure_release_folder_not_exists = ShortCircuitOperator( | ||
task_id="test_release_folder_exists", | ||
python_callable=lambda bucket, path: not common.check_gcp_folder_exists( | ||
bucket, path | ||
), | ||
Comment on lines
+104
to
+106
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be controversial, but I'm up for defending this decision: I don't like to write separate functions for testing the presence and for testing the absence of something. I like the idea to test something positive, because that's eaiser to understand eg. the tested folder is there -> returns However it implies that if we need to test the absence, we have to flip the boolean value. We can either flip the return value or make the function flexible to decide if we want the presence or the absence. I don't like the latter solution. This means if I'm using an airflow operator on a python callable, I need to use lambda. It might not be the nicest, but I think this is the right choice here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not controversial to me. Functions are meant to be reused. Thanks! |
||
op_kwargs={ | ||
"bucket": RELEASE_BUCKET_NAME, | ||
"path": f"releases/{RELEASE_VERSION}", | ||
}, | ||
) | ||
|
||
with DAG( | ||
dag_id=Path(__file__).stem, | ||
description="Open Targets Genetics ETL workflow", | ||
default_args=common.shared_dag_args, | ||
**common.shared_dag_kwargs, | ||
): | ||
# Compiling tasks for moving data to the right place: | ||
with TaskGroup(group_id="data_transfer") as data_transfer: | ||
# Defining the tasks to execute in the task group: | ||
[ | ||
GCSToGCSOperator( | ||
task_id=f"move_{data_name}", | ||
source_bucket=data["source_bucket"], | ||
source_object=data["source_object"], | ||
destination_bucket=data["destination_bucket"], | ||
destination_object=data["destination_object"], | ||
) | ||
for data_name, data in DATA_TO_MOVE.items() | ||
] | ||
|
||
with TaskGroup(group_id="genetics_etl") as genetics_etl: | ||
# Parse and define all steps and their prerequisites. | ||
tasks = {} | ||
steps = common.read_yaml_config(SOURCE_CONFIG_FILE_PATH) | ||
for step in steps: | ||
# Define task for the current step. | ||
step_id = step["id"] | ||
this_task = common.submit_step( | ||
cluster_name=CLUSTER_NAME, | ||
step_id=step_id, | ||
task_id=step_id, | ||
) | ||
# Chain prerequisites. | ||
tasks[step_id] = this_task | ||
for prerequisite in step.get("prerequisites", []): | ||
this_task.set_upstream(tasks[prerequisite]) | ||
|
||
common.generate_dag(cluster_name=CLUSTER_NAME, tasks=list(tasks.values())) | ||
|
||
# DAG description: | ||
( | ||
# Test that the release folder doesn't exist: | ||
ensure_release_folder_not_exists | ||
# Run data transfer: | ||
>> data_transfer | ||
# Once datasets are transferred, run the rest of the steps: | ||
>> genetics_etl | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to just point to our OTP release bucket?