From b9455b3a18f9fa73ada879cee43269cf306b0fb7 Mon Sep 17 00:00:00 2001 From: Calla Chennault <51501715+callachennault@users.noreply.github.com> Date: Tue, 24 Feb 2026 15:48:39 -0500 Subject: [PATCH 1/7] Add biobank merge code --- import-scripts/fetch-dmp-data-for-import.sh | 40 ++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/import-scripts/fetch-dmp-data-for-import.sh b/import-scripts/fetch-dmp-data-for-import.sh index 0bd6607f5..d608dc345 100755 --- a/import-scripts/fetch-dmp-data-for-import.sh +++ b/import-scripts/fetch-dmp-data-for-import.sh @@ -726,6 +726,25 @@ MY_FLOCK_FILEPATH="/data/portal-cron/cron-lock/fetch-dmp-data-for-import.lock" else echo "UNLINKED_ARCHER subset successful! Creating cancer type case lists..." echo $(date) + + # Pull biobank data from S3 + download_from_s3 "$MSK_ARCHER_DATA_HOME" "mskarcher/data_clinical_patient_biobank.txt" "mskimpact-databricks" + download_from_s3 "$MSK_ARCHER_DATA_HOME" "mskarcher/data_timeline_biobank_specimen.txt" "mskimpact-databricks" + + # Merge biobank clinical file + INPUT_CLINICAL_FILE="$MSK_ARCHER_DATA_HOME/data_clinical_patient.txt" + BIOBANK_CLINICAL_FILE="$MSK_ARCHER_DATA_HOME/data_clinical_patient_biobank.txt" + MERGED_CLINICAL_FILE="$MSK_ARCHER_DATA_HOME/data_clinical_patient_merged.txt" + $PYTHON3_BINARY $PORTAL_HOME/scripts/combine_files_py3.py -i "$INPUT_CLINICAL_FILE" "$BIOBANK_CLINICAL_FILE" -o "$MERGED_CLINICAL_FILE" -c "PATIENT_ID" -m left + if [ $? -gt 0 ] ; then + echo "Failed to merge $INPUT_CLINICAL_FILE and $BIOBANK_CLINICAL_FILE" + # TODO what should happen if this fails? will biobank clinical data just be skipped and not included in the portal import? + else + mv "$MERGED_CLINICAL_FILE" "$INPUT_CLINICAL_FILE" + # TODO should this be removed? + rm "$BIOBANK_CLINICAL_FILE" + fi + # add metadata headers and overrides before importing $PYTHON_BINARY $PORTAL_HOME/scripts/add_clinical_attribute_metadata_headers.py -s mskarcher -f $MSK_ARCHER_DATA_HOME/data_clinical* -i $PORTAL_HOME/scripts/cdm_metadata.json if [ $? -gt 0 ] ; then @@ -799,6 +818,25 @@ MY_FLOCK_FILEPATH="/data/portal-cron/cron-lock/fetch-dmp-data-for-import.lock" else echo "MSKSOLIDHEME merge successful! Creating cancer type case lists..." echo $(date) + + # Pull biobank data from S3 + download_from_s3 "$MSK_SOLID_HEME_DATA_HOME" "msk_solid_heme/data_clinical_patient_biobank.txt" "mskimpact-databricks" + download_from_s3 "$MSK_SOLID_HEME_DATA_HOME" "msk_solid_heme/data_timeline_biobank_specimen.txt" "mskimpact-databricks" + + # Merge biobank clinical file + INPUT_CLINICAL_FILE="$MSK_SOLID_HEME_DATA_HOME/data_clinical_patient.txt" + BIOBANK_CLINICAL_FILE="$MSK_SOLID_HEME_DATA_HOME/data_clinical_patient_biobank.txt" + MERGED_CLINICAL_FILE="$MSK_SOLID_HEME_DATA_HOME/data_clinical_patient_merged.txt" + $PYTHON3_BINARY $PORTAL_HOME/scripts/combine_files_py3.py -i "$INPUT_CLINICAL_FILE" "$BIOBANK_CLINICAL_FILE" -o "$MERGED_CLINICAL_FILE" -c "PATIENT_ID" -m left + if [ $? -gt 0 ] ; then + echo "Failed to merge $INPUT_CLINICAL_FILE and $BIOBANK_CLINICAL_FILE" + # TODO what should happen if this fails? will biobank clinical data just be skipped and not included in the portal import? + else + mv "$MERGED_CLINICAL_FILE" "$INPUT_CLINICAL_FILE" + # TODO should this be removed? + rm "$BIOBANK_CLINICAL_FILE" + fi + # add metadata headers and overrides before importing $PYTHON_BINARY $PORTAL_HOME/scripts/add_clinical_attribute_metadata_headers.py -s mskimpact -f $MSK_SOLID_HEME_DATA_HOME/data_clinical_sample.txt -i $PORTAL_HOME/scripts/cdm_metadata.json $PYTHON_BINARY $PORTAL_HOME/scripts/add_clinical_attribute_metadata_headers.py -s mskimpact -f $MSK_SOLID_HEME_DATA_HOME/data_clinical_patient.txt -i $PORTAL_HOME/scripts/cdm_metadata.json @@ -1204,7 +1242,7 @@ MY_FLOCK_FILEPATH="/data/portal-cron/cron-lock/fetch-dmp-data-for-import.lock" printTimeStampedDataProcessingStepMessage "push of dmp data updates to git repository" # check updated data back into git GIT_PUSH_FAIL=0 - cd $DMP_DATA_HOME ; $GIT_BINARY add ./*; git commit --amend --no-edit; $GIT_BINARY push origin --force + cd $DMP_DATA_HOME ; $GIT_BINARY add ./*; $GIT_BINARY commit --amend --no-edit; $GIT_BINARY push origin --force if [ $? -gt 0 ] ; then GIT_PUSH_FAIL=1 sendPreImportFailureMessageMskPipelineLogsSlack "GIT PUSH (dmp) :fire: - address ASAP!" From 223ef68ae35175cd1c710bbd3f67af4484ca7d02 Mon Sep 17 00:00:00 2001 From: Calla Chennault <51501715+callachennault@users.noreply.github.com> Date: Tue, 3 Mar 2026 16:45:11 -0500 Subject: [PATCH 2/7] fix download_from_S3 commands --- import-scripts/fetch-dmp-data-for-import.sh | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/import-scripts/fetch-dmp-data-for-import.sh b/import-scripts/fetch-dmp-data-for-import.sh index d608dc345..369dd593d 100755 --- a/import-scripts/fetch-dmp-data-for-import.sh +++ b/import-scripts/fetch-dmp-data-for-import.sh @@ -728,8 +728,8 @@ MY_FLOCK_FILEPATH="/data/portal-cron/cron-lock/fetch-dmp-data-for-import.lock" echo $(date) # Pull biobank data from S3 - download_from_s3 "$MSK_ARCHER_DATA_HOME" "mskarcher/data_clinical_patient_biobank.txt" "mskimpact-databricks" - download_from_s3 "$MSK_ARCHER_DATA_HOME" "mskarcher/data_timeline_biobank_specimen.txt" "mskimpact-databricks" + download_from_s3 "$MSK_ARCHER_DATA_HOME/data_clinical_patient_biobank.txt" "mskarcher/data_clinical_patient_biobank.txt" "mskimpact-databricks" + download_from_s3 "$MSK_ARCHER_DATA_HOME/data_timeline_biobank_specimen.txt" "mskarcher/data_timeline_biobank_specimen.txt" "mskimpact-databricks" # Merge biobank clinical file INPUT_CLINICAL_FILE="$MSK_ARCHER_DATA_HOME/data_clinical_patient.txt" @@ -741,7 +741,6 @@ MY_FLOCK_FILEPATH="/data/portal-cron/cron-lock/fetch-dmp-data-for-import.lock" # TODO what should happen if this fails? will biobank clinical data just be skipped and not included in the portal import? else mv "$MERGED_CLINICAL_FILE" "$INPUT_CLINICAL_FILE" - # TODO should this be removed? rm "$BIOBANK_CLINICAL_FILE" fi @@ -820,8 +819,8 @@ MY_FLOCK_FILEPATH="/data/portal-cron/cron-lock/fetch-dmp-data-for-import.lock" echo $(date) # Pull biobank data from S3 - download_from_s3 "$MSK_SOLID_HEME_DATA_HOME" "msk_solid_heme/data_clinical_patient_biobank.txt" "mskimpact-databricks" - download_from_s3 "$MSK_SOLID_HEME_DATA_HOME" "msk_solid_heme/data_timeline_biobank_specimen.txt" "mskimpact-databricks" + download_from_s3 "$MSK_SOLID_HEME_DATA_HOME/data_clinical_patient_biobank.txt" "msk_solid_heme/data_clinical_patient_biobank.txt" "mskimpact-databricks" + download_from_s3 "$MSK_SOLID_HEME_DATA_HOME/data_timeline_biobank_specimen.txt" "msk_solid_heme/data_timeline_biobank_specimen.txt" "mskimpact-databricks" # Merge biobank clinical file INPUT_CLINICAL_FILE="$MSK_SOLID_HEME_DATA_HOME/data_clinical_patient.txt" @@ -833,7 +832,6 @@ MY_FLOCK_FILEPATH="/data/portal-cron/cron-lock/fetch-dmp-data-for-import.lock" # TODO what should happen if this fails? will biobank clinical data just be skipped and not included in the portal import? else mv "$MERGED_CLINICAL_FILE" "$INPUT_CLINICAL_FILE" - # TODO should this be removed? rm "$BIOBANK_CLINICAL_FILE" fi From 0b79fecf83b77ab78effebe7f3f7342fb6ee4201 Mon Sep 17 00:00:00 2001 From: Manda Wilson <1458628+mandawilson@users.noreply.github.com> Date: Thu, 18 Jun 2026 14:44:06 +0100 Subject: [PATCH 3/7] Skip biobank merge non-fatally when S3 download or combine fails. Move biobank download/merge into fetch-dmp-data-for-import.sh with best-effort aws s3 cp so a missing biobank file does not abort the import, and set BIOBANK_SKIPPED flags when biobank data is omitted. --- import-scripts/fetch-dmp-data-for-import.sh | 93 ++++++++++++++------- 1 file changed, 61 insertions(+), 32 deletions(-) diff --git a/import-scripts/fetch-dmp-data-for-import.sh b/import-scripts/fetch-dmp-data-for-import.sh index 39f3ae603..4ea4ee076 100755 --- a/import-scripts/fetch-dmp-data-for-import.sh +++ b/import-scripts/fetch-dmp-data-for-import.sh @@ -13,6 +13,63 @@ MY_FLOCK_FILEPATH="/data/portal-cron/cron-lock/fetch-dmp-data-for-import.lock" # localize global variables / jar names and functions source $PORTAL_HOME/scripts/dmp-import-vars-functions.sh + function download_biobank_file_from_s3() { + local local_path="$1" + local s3_key="$2" + local bucket_name="$3" + + mkdir -p "$(dirname "$local_path")" + $PORTAL_HOME/scripts/authenticate_service_account.sh eks + aws s3 cp "s3://${bucket_name}/${s3_key}" "$local_path" --profile saml + return $? + } + + # Download biobank patient clinical and timeline data from S3 and merge into the study. + # On download or clinical merge failure, skip biobank for this import without failing the pipeline. + # Sets the caller's skipped_flag variable to 1 when any biobank step is skipped. + function merge_biobank_clinical_data() { + local study_data_home="$1" + local s3_study_prefix="$2" + local bucket_name="${3:-mskimpact-databricks}" + local skipped_flag="${4:-BIOBANK_SKIPPED}" + + local input_clinical_file="$study_data_home/data_clinical_patient.txt" + local biobank_clinical_file="$study_data_home/data_clinical_patient_biobank.txt" + local biobank_timeline_file="$study_data_home/data_timeline_biobank_specimen.txt" + local merged_clinical_file="$study_data_home/data_clinical_patient_merged.txt" + + eval "$skipped_flag=0" + + rm -f "$biobank_clinical_file" "$biobank_timeline_file" "$merged_clinical_file" + + if ! download_biobank_file_from_s3 "$biobank_clinical_file" "${s3_study_prefix}/data_clinical_patient_biobank.txt" "$bucket_name"; then + echo "`date`: Warning: failed to download biobank patient clinical data from S3 for ${s3_study_prefix}; skipping biobank clinical and timeline for this import." + eval "$skipped_flag=1" + return 0 + fi + + $PYTHON3_BINARY $PORTAL_HOME/scripts/combine_files_py3.py -i "$input_clinical_file" "$biobank_clinical_file" -o "$merged_clinical_file" -c "PATIENT_ID" -m left + if [ $? -gt 0 ]; then + echo "`date`: Warning: failed to merge biobank patient clinical data for ${s3_study_prefix}; skipping biobank clinical and timeline for this import." + rm -f "$biobank_clinical_file" "$biobank_timeline_file" "$merged_clinical_file" + eval "$skipped_flag=1" + return 0 + fi + + mv "$merged_clinical_file" "$input_clinical_file" + rm -f "$biobank_clinical_file" + + if ! download_biobank_file_from_s3 "$biobank_timeline_file" "${s3_study_prefix}/data_timeline_biobank_specimen.txt" "$bucket_name"; then + echo "`date`: Warning: failed to download biobank timeline data from S3 for ${s3_study_prefix}; skipping biobank timeline for this import." + rm -f "$biobank_timeline_file" + eval "$skipped_flag=1" + return 0 + fi + + echo "`date`: Biobank patient clinical and timeline data ready for ${s3_study_prefix}." + return 0 + } + ## STATUS FLAGS # Flags indicating whether a study can be updated @@ -40,6 +97,8 @@ MY_FLOCK_FILEPATH="/data/portal-cron/cron-lock/fetch-dmp-data-for-import.lock" UNLINKED_ARCHER_SUBSET_FAIL=0 MIXEDPACT_MERGE_FAIL=0 MSK_SOLID_HEME_MERGE_FAIL=0 + BIOBANK_SKIPPED_ARCHER=0 + BIOBANK_SKIPPED_MSK_SOLID_HEME=0 MSK_KINGS_SUBSET_FAIL=0 MSK_QUEENS_SUBSET_FAIL=0 MSK_LEHIGH_SUBSET_FAIL=0 @@ -734,22 +793,7 @@ MY_FLOCK_FILEPATH="/data/portal-cron/cron-lock/fetch-dmp-data-for-import.lock" echo "UNLINKED_ARCHER subset successful! Creating cancer type case lists..." echo $(date) - # Pull biobank data from S3 - download_from_s3 "$MSK_ARCHER_DATA_HOME/data_clinical_patient_biobank.txt" "mskarcher/data_clinical_patient_biobank.txt" "mskimpact-databricks" - download_from_s3 "$MSK_ARCHER_DATA_HOME/data_timeline_biobank_specimen.txt" "mskarcher/data_timeline_biobank_specimen.txt" "mskimpact-databricks" - - # Merge biobank clinical file - INPUT_CLINICAL_FILE="$MSK_ARCHER_DATA_HOME/data_clinical_patient.txt" - BIOBANK_CLINICAL_FILE="$MSK_ARCHER_DATA_HOME/data_clinical_patient_biobank.txt" - MERGED_CLINICAL_FILE="$MSK_ARCHER_DATA_HOME/data_clinical_patient_merged.txt" - $PYTHON3_BINARY $PORTAL_HOME/scripts/combine_files_py3.py -i "$INPUT_CLINICAL_FILE" "$BIOBANK_CLINICAL_FILE" -o "$MERGED_CLINICAL_FILE" -c "PATIENT_ID" -m left - if [ $? -gt 0 ] ; then - echo "Failed to merge $INPUT_CLINICAL_FILE and $BIOBANK_CLINICAL_FILE" - # TODO what should happen if this fails? will biobank clinical data just be skipped and not included in the portal import? - else - mv "$MERGED_CLINICAL_FILE" "$INPUT_CLINICAL_FILE" - rm "$BIOBANK_CLINICAL_FILE" - fi + merge_biobank_clinical_data "$MSK_ARCHER_DATA_HOME" "mskarcher" "mskimpact-databricks" BIOBANK_SKIPPED_ARCHER # add metadata headers and overrides before importing $PYTHON_BINARY $PORTAL_HOME/scripts/add_clinical_attribute_metadata_headers.py -s mskarcher -f $MSK_ARCHER_DATA_HOME/data_clinical* -i $PORTAL_HOME/scripts/cdm_metadata.json @@ -825,22 +869,7 @@ MY_FLOCK_FILEPATH="/data/portal-cron/cron-lock/fetch-dmp-data-for-import.lock" echo "MSKSOLIDHEME merge successful! Creating cancer type case lists..." echo $(date) - # Pull biobank data from S3 - download_from_s3 "$MSK_SOLID_HEME_DATA_HOME/data_clinical_patient_biobank.txt" "msk_solid_heme/data_clinical_patient_biobank.txt" "mskimpact-databricks" - download_from_s3 "$MSK_SOLID_HEME_DATA_HOME/data_timeline_biobank_specimen.txt" "msk_solid_heme/data_timeline_biobank_specimen.txt" "mskimpact-databricks" - - # Merge biobank clinical file - INPUT_CLINICAL_FILE="$MSK_SOLID_HEME_DATA_HOME/data_clinical_patient.txt" - BIOBANK_CLINICAL_FILE="$MSK_SOLID_HEME_DATA_HOME/data_clinical_patient_biobank.txt" - MERGED_CLINICAL_FILE="$MSK_SOLID_HEME_DATA_HOME/data_clinical_patient_merged.txt" - $PYTHON3_BINARY $PORTAL_HOME/scripts/combine_files_py3.py -i "$INPUT_CLINICAL_FILE" "$BIOBANK_CLINICAL_FILE" -o "$MERGED_CLINICAL_FILE" -c "PATIENT_ID" -m left - if [ $? -gt 0 ] ; then - echo "Failed to merge $INPUT_CLINICAL_FILE and $BIOBANK_CLINICAL_FILE" - # TODO what should happen if this fails? will biobank clinical data just be skipped and not included in the portal import? - else - mv "$MERGED_CLINICAL_FILE" "$INPUT_CLINICAL_FILE" - rm "$BIOBANK_CLINICAL_FILE" - fi + merge_biobank_clinical_data "$MSK_SOLID_HEME_DATA_HOME" "msk_solid_heme" "mskimpact-databricks" BIOBANK_SKIPPED_MSK_SOLID_HEME # add metadata headers and overrides before importing $PYTHON_BINARY $PORTAL_HOME/scripts/add_clinical_attribute_metadata_headers.py -s mskimpact -f $MSK_SOLID_HEME_DATA_HOME/data_clinical_sample.txt -i $PORTAL_HOME/scripts/cdm_metadata.json From 8461bc38eb4e20c2b6c69f1ed6b464b6367d68f2 Mon Sep 17 00:00:00 2001 From: Manda Wilson <1458628+mandawilson@users.noreply.github.com> Date: Thu, 18 Jun 2026 14:53:29 +0100 Subject: [PATCH 4/7] Add try_upload/download S3 helpers while preserving fatal wrappers. Extract return-based try_upload_to_s3 and try_download_from_s3 so biobank fetch can skip failed downloads without changing existing upload_to_s3 and download_from_s3 behavior. --- import-scripts/fetch-dmp-data-for-import.sh | 5 +- import-scripts/s3_functions.sh | 52 ++++++++++++++------- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/import-scripts/fetch-dmp-data-for-import.sh b/import-scripts/fetch-dmp-data-for-import.sh index 4ea4ee076..aebd9f8a9 100755 --- a/import-scripts/fetch-dmp-data-for-import.sh +++ b/import-scripts/fetch-dmp-data-for-import.sh @@ -19,9 +19,8 @@ MY_FLOCK_FILEPATH="/data/portal-cron/cron-lock/fetch-dmp-data-for-import.lock" local bucket_name="$3" mkdir -p "$(dirname "$local_path")" - $PORTAL_HOME/scripts/authenticate_service_account.sh eks - aws s3 cp "s3://${bucket_name}/${s3_key}" "$local_path" --profile saml - return $? + touch "$local_path" + try_download_from_s3 "$local_path" "$s3_key" "$bucket_name" } # Download biobank patient clinical and timeline data from S3 and merge into the study. diff --git a/import-scripts/s3_functions.sh b/import-scripts/s3_functions.sh index 3738306a5..06bc262de 100644 --- a/import-scripts/s3_functions.sh +++ b/import-scripts/s3_functions.sh @@ -5,10 +5,11 @@ if ! [ -n "$PORTAL_HOME" ] ; then exit 1 fi -# Uploads a file or directory to an S3 bucket (from local to S3) -# If a directory is provided, files removed locally will be deleted from S3 -# PATH_TO_UPLOAD must end in the same path as PATH_IN_S3, or the script exits with an error -function upload_to_s3() { +# Uploads a file or directory to an S3 bucket (from local to S3). +# If a directory is provided, files removed locally will be deleted from S3. +# PATH_TO_UPLOAD must end in the same path as PATH_IN_S3, or the script exits with an error. +# Returns 0 on success, 1 on failure (does not exit the shell). +function try_upload_to_s3() { PATH_TO_UPLOAD="$1" PATH_IN_S3="$2" BUCKET_NAME="$3" @@ -16,7 +17,7 @@ function upload_to_s3() { # Check if path exists if [ ! -e "$PATH_TO_UPLOAD" ]; then echo "`date`: Path '$PATH_TO_UPLOAD' does not exist, exiting..." - exit 1 + return 1 fi # Normalize paths @@ -28,7 +29,7 @@ function upload_to_s3() { if [ -n "$PATH_IN_S3_CLEAN" ]; then if [[ "$PATH_TO_UPLOAD_ABS" != *"/$PATH_IN_S3_CLEAN" && "$PATH_TO_UPLOAD_ABS" != "$PATH_IN_S3_CLEAN" ]]; then echo "`date`: ERROR – PATH_IN_S3 ('$PATH_IN_S3') must exactly match the trailing path of '$PATH_TO_UPLOAD_ABS'. Exiting..." - exit 1 + return 1 fi fi @@ -56,19 +57,29 @@ function upload_to_s3() { --profile saml else echo "`date`: '$PATH_TO_UPLOAD' is neither a file nor a directory, exiting..." - exit 1 + return 1 fi if [ $? -ne 0 ]; then echo "`date`: Failed to upload '$PATH_TO_UPLOAD' to S3, exiting..." + return 1 + fi +} + +# Uploads a file or directory to an S3 bucket (from local to S3) +# If a directory is provided, files removed locally will be deleted from S3 +# PATH_TO_UPLOAD must end in the same path as PATH_IN_S3, or the script exits with an error +function upload_to_s3() { + if ! try_upload_to_s3 "$1" "$2" "$3"; then exit 1 fi } -# Syncs a file or directory from S3 to a local path -# Files not in S3 will be removed from the local destination (for directories) -# PATH_TO_OVERWRITE must end in PATH_IN_S3 (if provided), and cannot be "/" -function download_from_s3() { +# Syncs a file or directory from S3 to a local path. +# Files not in S3 will be removed from the local destination (for directories). +# PATH_TO_OVERWRITE must end in PATH_IN_S3 (if provided), and cannot be "/". +# Returns 0 on success, 1 on failure (does not exit the shell). +function try_download_from_s3() { PATH_TO_OVERWRITE="$1" PATH_IN_S3="$2" BUCKET_NAME="$3" @@ -76,18 +87,18 @@ function download_from_s3() { # Normalize local path if [ -z "$PATH_TO_OVERWRITE" ]; then echo "`date`: PATH_TO_OVERWRITE is empty, exiting..." - exit 1 + return 1 fi PATH_TO_OVERWRITE_ABS=$(realpath "$PATH_TO_OVERWRITE" 2>/dev/null) if [ $? -ne 0 ]; then echo "`date`: '$PATH_TO_OVERWRITE' does not exist, exiting..." - exit 1 + return 1 fi if [ "$PATH_TO_OVERWRITE_ABS" == "/" ]; then echo "`date`: Refusing to sync to root directory '/', exiting..." - exit 1 + return 1 fi # Clean PATH_IN_S3 @@ -97,7 +108,7 @@ function download_from_s3() { if [ -n "$PATH_IN_S3_CLEAN" ]; then if [[ "$PATH_TO_OVERWRITE_ABS" != *"/$PATH_IN_S3_CLEAN" && "$PATH_TO_OVERWRITE_ABS" != "$PATH_IN_S3_CLEAN" ]]; then echo "`date`: ERROR – PATH_IN_S3 ('$PATH_IN_S3') must exactly match the trailing path of '$PATH_TO_OVERWRITE_ABS'. Exiting..." - exit 1 + return 1 fi fi @@ -129,11 +140,20 @@ function download_from_s3() { else echo "`date`: '$PATH_TO_OVERWRITE_ABS' is neither a file nor a directory, exiting..." - exit 1 + return 1 fi if [ $? -ne 0 ]; then echo "`date`: Failed to download from '$S3_SOURCE', exiting..." + return 1 + fi +} + +# Syncs a file or directory from S3 to a local path +# Files not in S3 will be removed from the local destination (for directories) +# PATH_TO_OVERWRITE must end in PATH_IN_S3 (if provided), and cannot be "/" +function download_from_s3() { + if ! try_download_from_s3 "$1" "$2" "$3"; then exit 1 fi } From a184bab88abb0b356ff2f84fb1d86cf05d3affab Mon Sep 17 00:00:00 2001 From: Manda Wilson <1458628+mandawilson@users.noreply.github.com> Date: Thu, 18 Jun 2026 14:57:21 +0100 Subject: [PATCH 5/7] Simplify biobank merge to call try_download_from_s3 directly. Remove unused BIOBANK_SKIPPED flags and the download_biobank_file_from_s3 wrapper; document rm/touch before biobank downloads in merge_biobank_clinical_data. --- import-scripts/fetch-dmp-data-for-import.sh | 32 ++++++--------------- 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/import-scripts/fetch-dmp-data-for-import.sh b/import-scripts/fetch-dmp-data-for-import.sh index aebd9f8a9..04b349fc4 100755 --- a/import-scripts/fetch-dmp-data-for-import.sh +++ b/import-scripts/fetch-dmp-data-for-import.sh @@ -13,37 +13,26 @@ MY_FLOCK_FILEPATH="/data/portal-cron/cron-lock/fetch-dmp-data-for-import.lock" # localize global variables / jar names and functions source $PORTAL_HOME/scripts/dmp-import-vars-functions.sh - function download_biobank_file_from_s3() { - local local_path="$1" - local s3_key="$2" - local bucket_name="$3" - - mkdir -p "$(dirname "$local_path")" - touch "$local_path" - try_download_from_s3 "$local_path" "$s3_key" "$bucket_name" - } - # Download biobank patient clinical and timeline data from S3 and merge into the study. # On download or clinical merge failure, skip biobank for this import without failing the pipeline. - # Sets the caller's skipped_flag variable to 1 when any biobank step is skipped. function merge_biobank_clinical_data() { local study_data_home="$1" local s3_study_prefix="$2" local bucket_name="${3:-mskimpact-databricks}" - local skipped_flag="${4:-BIOBANK_SKIPPED}" local input_clinical_file="$study_data_home/data_clinical_patient.txt" local biobank_clinical_file="$study_data_home/data_clinical_patient_biobank.txt" local biobank_timeline_file="$study_data_home/data_timeline_biobank_specimen.txt" local merged_clinical_file="$study_data_home/data_clinical_patient_merged.txt" - eval "$skipped_flag=0" - + # Remove stale biobank files so a failed or skipped run does not leave old timeline data + # for merge-cdm-timeline-files.sh. try_download_from_s3 requires the local file to exist; + # touch a placeholder before each download. rm -f "$biobank_clinical_file" "$biobank_timeline_file" "$merged_clinical_file" + touch "$biobank_clinical_file" - if ! download_biobank_file_from_s3 "$biobank_clinical_file" "${s3_study_prefix}/data_clinical_patient_biobank.txt" "$bucket_name"; then + if ! try_download_from_s3 "$biobank_clinical_file" "${s3_study_prefix}/data_clinical_patient_biobank.txt" "$bucket_name"; then echo "`date`: Warning: failed to download biobank patient clinical data from S3 for ${s3_study_prefix}; skipping biobank clinical and timeline for this import." - eval "$skipped_flag=1" return 0 fi @@ -51,17 +40,16 @@ MY_FLOCK_FILEPATH="/data/portal-cron/cron-lock/fetch-dmp-data-for-import.lock" if [ $? -gt 0 ]; then echo "`date`: Warning: failed to merge biobank patient clinical data for ${s3_study_prefix}; skipping biobank clinical and timeline for this import." rm -f "$biobank_clinical_file" "$biobank_timeline_file" "$merged_clinical_file" - eval "$skipped_flag=1" return 0 fi mv "$merged_clinical_file" "$input_clinical_file" rm -f "$biobank_clinical_file" - if ! download_biobank_file_from_s3 "$biobank_timeline_file" "${s3_study_prefix}/data_timeline_biobank_specimen.txt" "$bucket_name"; then + touch "$biobank_timeline_file" + if ! try_download_from_s3 "$biobank_timeline_file" "${s3_study_prefix}/data_timeline_biobank_specimen.txt" "$bucket_name"; then echo "`date`: Warning: failed to download biobank timeline data from S3 for ${s3_study_prefix}; skipping biobank timeline for this import." rm -f "$biobank_timeline_file" - eval "$skipped_flag=1" return 0 fi @@ -96,8 +84,6 @@ MY_FLOCK_FILEPATH="/data/portal-cron/cron-lock/fetch-dmp-data-for-import.lock" UNLINKED_ARCHER_SUBSET_FAIL=0 MIXEDPACT_MERGE_FAIL=0 MSK_SOLID_HEME_MERGE_FAIL=0 - BIOBANK_SKIPPED_ARCHER=0 - BIOBANK_SKIPPED_MSK_SOLID_HEME=0 MSK_KINGS_SUBSET_FAIL=0 MSK_QUEENS_SUBSET_FAIL=0 MSK_LEHIGH_SUBSET_FAIL=0 @@ -792,7 +778,7 @@ MY_FLOCK_FILEPATH="/data/portal-cron/cron-lock/fetch-dmp-data-for-import.lock" echo "UNLINKED_ARCHER subset successful! Creating cancer type case lists..." echo $(date) - merge_biobank_clinical_data "$MSK_ARCHER_DATA_HOME" "mskarcher" "mskimpact-databricks" BIOBANK_SKIPPED_ARCHER + merge_biobank_clinical_data "$MSK_ARCHER_DATA_HOME" "mskarcher" "mskimpact-databricks" # add metadata headers and overrides before importing $PYTHON_BINARY $PORTAL_HOME/scripts/add_clinical_attribute_metadata_headers.py -s mskarcher -f $MSK_ARCHER_DATA_HOME/data_clinical* -i $PORTAL_HOME/scripts/cdm_metadata.json @@ -868,7 +854,7 @@ MY_FLOCK_FILEPATH="/data/portal-cron/cron-lock/fetch-dmp-data-for-import.lock" echo "MSKSOLIDHEME merge successful! Creating cancer type case lists..." echo $(date) - merge_biobank_clinical_data "$MSK_SOLID_HEME_DATA_HOME" "msk_solid_heme" "mskimpact-databricks" BIOBANK_SKIPPED_MSK_SOLID_HEME + merge_biobank_clinical_data "$MSK_SOLID_HEME_DATA_HOME" "msk_solid_heme" "mskimpact-databricks" # add metadata headers and overrides before importing $PYTHON_BINARY $PORTAL_HOME/scripts/add_clinical_attribute_metadata_headers.py -s mskimpact -f $MSK_SOLID_HEME_DATA_HOME/data_clinical_sample.txt -i $PORTAL_HOME/scripts/cdm_metadata.json From 3a15081df75c20f53f65697328b0a318bdd09f85 Mon Sep 17 00:00:00 2001 From: Manda Wilson <1458628+mandawilson@users.noreply.github.com> Date: Thu, 18 Jun 2026 15:23:53 +0100 Subject: [PATCH 6/7] Use bulk-synced biobank files instead of re-downloading from S3. Merge biobank clinical data when the file is already present from downloadFromS3AllStudies; keep timeline on clinical merge failure and clean up the merged file only when combine fails. --- import-scripts/fetch-dmp-data-for-import.sh | 48 ++++++++------------- 1 file changed, 17 insertions(+), 31 deletions(-) diff --git a/import-scripts/fetch-dmp-data-for-import.sh b/import-scripts/fetch-dmp-data-for-import.sh index 04b349fc4..ecaf157df 100755 --- a/import-scripts/fetch-dmp-data-for-import.sh +++ b/import-scripts/fetch-dmp-data-for-import.sh @@ -13,47 +13,33 @@ MY_FLOCK_FILEPATH="/data/portal-cron/cron-lock/fetch-dmp-data-for-import.lock" # localize global variables / jar names and functions source $PORTAL_HOME/scripts/dmp-import-vars-functions.sh - # Download biobank patient clinical and timeline data from S3 and merge into the study. - # On download or clinical merge failure, skip biobank for this import without failing the pipeline. + # Merge biobank patient clinical data when present (already synced via downloadFromS3AllStudies). + # Clinical merge failure is non-fatal; an existing biobank timeline file is kept for CDM timeline merge. function merge_biobank_clinical_data() { local study_data_home="$1" - local s3_study_prefix="$2" - local bucket_name="${3:-mskimpact-databricks}" local input_clinical_file="$study_data_home/data_clinical_patient.txt" local biobank_clinical_file="$study_data_home/data_clinical_patient_biobank.txt" local biobank_timeline_file="$study_data_home/data_timeline_biobank_specimen.txt" local merged_clinical_file="$study_data_home/data_clinical_patient_merged.txt" - # Remove stale biobank files so a failed or skipped run does not leave old timeline data - # for merge-cdm-timeline-files.sh. try_download_from_s3 requires the local file to exist; - # touch a placeholder before each download. - rm -f "$biobank_clinical_file" "$biobank_timeline_file" "$merged_clinical_file" - touch "$biobank_clinical_file" - - if ! try_download_from_s3 "$biobank_clinical_file" "${s3_study_prefix}/data_clinical_patient_biobank.txt" "$bucket_name"; then - echo "`date`: Warning: failed to download biobank patient clinical data from S3 for ${s3_study_prefix}; skipping biobank clinical and timeline for this import." - return 0 - fi - - $PYTHON3_BINARY $PORTAL_HOME/scripts/combine_files_py3.py -i "$input_clinical_file" "$biobank_clinical_file" -o "$merged_clinical_file" -c "PATIENT_ID" -m left - if [ $? -gt 0 ]; then - echo "`date`: Warning: failed to merge biobank patient clinical data for ${s3_study_prefix}; skipping biobank clinical and timeline for this import." - rm -f "$biobank_clinical_file" "$biobank_timeline_file" "$merged_clinical_file" - return 0 + if [ -f "$biobank_clinical_file" ]; then + $PYTHON3_BINARY $PORTAL_HOME/scripts/combine_files_py3.py -i "$input_clinical_file" "$biobank_clinical_file" -o "$merged_clinical_file" -c "PATIENT_ID" -m left + if [ $? -gt 0 ]; then + echo "`date`: Warning: failed to merge biobank patient clinical data for ${study_data_home}; skipping biobank clinical merge for this import." + rm -f "$merged_clinical_file" + else + mv "$merged_clinical_file" "$input_clinical_file" + rm -f "$biobank_clinical_file" + fi + else + echo "`date`: Warning: biobank patient clinical file not found in ${study_data_home}; skipping biobank clinical merge for this import." fi - mv "$merged_clinical_file" "$input_clinical_file" - rm -f "$biobank_clinical_file" - - touch "$biobank_timeline_file" - if ! try_download_from_s3 "$biobank_timeline_file" "${s3_study_prefix}/data_timeline_biobank_specimen.txt" "$bucket_name"; then - echo "`date`: Warning: failed to download biobank timeline data from S3 for ${s3_study_prefix}; skipping biobank timeline for this import." - rm -f "$biobank_timeline_file" - return 0 + if [ -f "$biobank_timeline_file" ]; then + echo "`date`: Biobank timeline data present in ${study_data_home}." fi - echo "`date`: Biobank patient clinical and timeline data ready for ${s3_study_prefix}." return 0 } @@ -778,7 +764,7 @@ MY_FLOCK_FILEPATH="/data/portal-cron/cron-lock/fetch-dmp-data-for-import.lock" echo "UNLINKED_ARCHER subset successful! Creating cancer type case lists..." echo $(date) - merge_biobank_clinical_data "$MSK_ARCHER_DATA_HOME" "mskarcher" "mskimpact-databricks" + merge_biobank_clinical_data "$MSK_ARCHER_DATA_HOME" # add metadata headers and overrides before importing $PYTHON_BINARY $PORTAL_HOME/scripts/add_clinical_attribute_metadata_headers.py -s mskarcher -f $MSK_ARCHER_DATA_HOME/data_clinical* -i $PORTAL_HOME/scripts/cdm_metadata.json @@ -854,7 +840,7 @@ MY_FLOCK_FILEPATH="/data/portal-cron/cron-lock/fetch-dmp-data-for-import.lock" echo "MSKSOLIDHEME merge successful! Creating cancer type case lists..." echo $(date) - merge_biobank_clinical_data "$MSK_SOLID_HEME_DATA_HOME" "msk_solid_heme" "mskimpact-databricks" + merge_biobank_clinical_data "$MSK_SOLID_HEME_DATA_HOME" # add metadata headers and overrides before importing $PYTHON_BINARY $PORTAL_HOME/scripts/add_clinical_attribute_metadata_headers.py -s mskimpact -f $MSK_SOLID_HEME_DATA_HOME/data_clinical_sample.txt -i $PORTAL_HOME/scripts/cdm_metadata.json From 10f99b172e8a9dc132dd4af7f590a9e157257497 Mon Sep 17 00:00:00 2001 From: Manda Wilson <1458628+mandawilson@users.noreply.github.com> Date: Thu, 18 Jun 2026 18:32:56 +0100 Subject: [PATCH 7/7] Document arguments for S3 upload and download helpers. Add Args blocks to upload_to_s3, download_from_s3, and the try_* wrappers so callers can see path, S3 prefix, and bucket parameters. --- import-scripts/s3_functions.sh | 42 +++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/import-scripts/s3_functions.sh b/import-scripts/s3_functions.sh index 06bc262de..46a9f2ab4 100644 --- a/import-scripts/s3_functions.sh +++ b/import-scripts/s3_functions.sh @@ -7,8 +7,14 @@ fi # Uploads a file or directory to an S3 bucket (from local to S3). # If a directory is provided, files removed locally will be deleted from S3. -# PATH_TO_UPLOAD must end in the same path as PATH_IN_S3, or the script exits with an error. # Returns 0 on success, 1 on failure (does not exit the shell). +# +# Args: +# $1 PATH_TO_UPLOAD Local file or directory to upload (must exist). +# $2 PATH_IN_S3 S3 key prefix within the bucket (no s3:// prefix). Must match +# the trailing path of PATH_TO_UPLOAD, unless empty to sync the +# bucket root (e.g. upload_to_s3 "$DMP_DATA_HOME" "" "mskimpact-databricks"). +# $3 BUCKET_NAME S3 bucket name (e.g. mskimpact-databricks). function try_upload_to_s3() { PATH_TO_UPLOAD="$1" PATH_IN_S3="$2" @@ -66,9 +72,16 @@ function try_upload_to_s3() { fi } -# Uploads a file or directory to an S3 bucket (from local to S3) -# If a directory is provided, files removed locally will be deleted from S3 -# PATH_TO_UPLOAD must end in the same path as PATH_IN_S3, or the script exits with an error +# Uploads a file or directory to an S3 bucket (from local to S3). +# If a directory is provided, files removed locally will be deleted from S3. +# Exits 1 on failure. +# +# Args: +# $1 PATH_TO_UPLOAD Local file or directory to upload (must exist). +# $2 PATH_IN_S3 S3 key prefix within the bucket (no s3:// prefix). Must match +# the trailing path of PATH_TO_UPLOAD, unless empty to sync the +# bucket root (e.g. upload_to_s3 "$DMP_DATA_HOME" "" "mskimpact-databricks"). +# $3 BUCKET_NAME S3 bucket name (e.g. mskimpact-databricks). function upload_to_s3() { if ! try_upload_to_s3 "$1" "$2" "$3"; then exit 1 @@ -77,8 +90,14 @@ function upload_to_s3() { # Syncs a file or directory from S3 to a local path. # Files not in S3 will be removed from the local destination (for directories). -# PATH_TO_OVERWRITE must end in PATH_IN_S3 (if provided), and cannot be "/". # Returns 0 on success, 1 on failure (does not exit the shell). +# +# Args: +# $1 PATH_TO_OVERWRITE Local file or directory to sync into (must already exist; cannot be "/"). +# $2 PATH_IN_S3 S3 key prefix within the bucket (no s3:// prefix). Must match +# the trailing path of PATH_TO_OVERWRITE, unless empty to sync the +# bucket root (e.g. download_from_s3 "$DMP_DATA_HOME" "" "mskimpact-databricks"). +# $3 BUCKET_NAME S3 bucket name (e.g. mskimpact-databricks). function try_download_from_s3() { PATH_TO_OVERWRITE="$1" PATH_IN_S3="$2" @@ -149,9 +168,16 @@ function try_download_from_s3() { fi } -# Syncs a file or directory from S3 to a local path -# Files not in S3 will be removed from the local destination (for directories) -# PATH_TO_OVERWRITE must end in PATH_IN_S3 (if provided), and cannot be "/" +# Syncs a file or directory from S3 to a local path. +# Files not in S3 will be removed from the local destination (for directories). +# Exits 1 on failure. +# +# Args: +# $1 PATH_TO_OVERWRITE Local file or directory to sync into (must already exist; cannot be "/"). +# $2 PATH_IN_S3 S3 key prefix within the bucket (no s3:// prefix). Must match +# the trailing path of PATH_TO_OVERWRITE, unless empty to sync the +# bucket root (e.g. download_from_s3 "$DMP_DATA_HOME" "" "mskimpact-databricks"). +# $3 BUCKET_NAME S3 bucket name (e.g. mskimpact-databricks). function download_from_s3() { if ! try_download_from_s3 "$1" "$2" "$3"; then exit 1