ACTIN-2312 Align shared data buckets between EMC and NKI #112

andreia-hartwig · 2025-07-21T11:49:55Z

This PR includes:

Align shared data buckets between EMC and NKI, using only shared-data-external, with the content format of shared-data
Remove clinical.json from shared-data-external as it contains the same as patient_record.json
Remove creation and usage of evaluation summary and details

Related PR's:
https://github.com/hartwigmedical/actin-pipelines/pull/271
hartwigmedical/actin#1266
https://github.com/hartwigmedical/actin-infrastructure/pull/253

…shared-data-external, with the content format of shared-data. Remove clinical.json from shared-data-external as it contains the same as patient_record.json Remove creation and usage of evaluation summary and details.

ninajacobs · 2025-07-21T13:26:18Z

actin/local/operational/archive_emc_actin_orange_data_after_sharing

@@ -16,11 +16,11 @@ if [[ -z "${namespace}" ]]; then
 fi


Do we still need this script?

This script creates the files in the orange subfolder. We agreed with Korneel to keep it.

oki! I couldn't remember if we wanted to keep it.

maybe here in the title and logging we can also make it clearer that it is pushed to the shared external bucket and not directly to an archival bucket? Eg 'Copying' instead of 'Archiving' in logging?

ninajacobs · 2025-07-21T13:31:49Z

actin/local/operational/share_report_and_clinical_and_matching_snapshot_with_nki

@@ -1,53 +1,37 @@
 #!/usr/bin/env bash


maybe we should rename the EMC and NKI scripts, to have a similar name? I think that would be clearer

@ninajacobs Not quite related to this PR, but at this point can we delete the pull and approve scripts for the NKI feed? I can take care of that separately.

@jbartletthmf you're asking since we're using the UI now right? If you don't see a reason why we should need these anymore in the future, then I think so, yes! I indeed haven't used the scripts for this the last times

@ninajacobs These comments are confusing, are you saying you don't use share_report_and_clinical_and_matching_snapshot_with_nki or pull and approve scripts for the NKI?

@andreia-hartwig We're talking about pull_transformed_nki_feed and approve_transformed_nki_feed, which I'll delete separate from this PR. Sorry for the confusion.

I propose to rename share_report_and_clinical_and_matching_snapshot_with_nki and it's EMC counterpart to something similar

btw the EMC script has 'archiving' in the name and I don't think it archives (anymore), only sharing data to external shared bucket? how about share_actin_report_and_relevant_data_with_emc and share_actin_report_and_relevant_data_with_nki ?

ninajacobs · 2025-07-21T15:56:47Z

actin/local/operational/share_actin_data_and_report_with_emc

@@ -38,7 +38,7 @@ if [[ -z "${orange_no_germline_json}" && -z "${orange_no_germline_pdf}" ]]; then
    warn "Skipping archiving of ORANGE data for ${patient} as they do not exist"


finding one more 'archiving' that can be replaced by 'copying' :)

Like the new names, LGTM!

Will you update the EMC and NKI Confluence pages too?

only once it's merged

…t to a versioned folder.

andreia-hartwig · 2025-07-24T13:20:02Z

@jbartletthmf @ninajacobs Please review the latest commit where I introduce a latest folder, new files to a latest folder moving the previous latest are moved to a versioned folder.

jbartletthmf

It's unfortunate that Google storage doesn't support symlinks or atomic move operations, because having to move the latest folder every time there's an update isn't ideal. Given those constraints, I don't see a better way to have a constant path for the latest data. Some alternatives would be to:

store the latest version in a separate file at the top level of the directory, so consumers of the data could read that file first to determine the path
resolve the latest path at access time by listing the directories

Both of those options simplify updates but complicate reads. Since we update in one place and may read from many, it probably makes sense to stick with the current approach.

jbartletthmf · 2025-07-24T13:52:40Z

actin/local/operational/share_actin_data_and_report_with_emc

+if [[ "$version_folder" != "1" ]]; then
+  info "Moving ACTIN data from $shared_actin_data_external_latest_uri to $shared_actin_data_external_uri"
+  gsutil -m mv "$shared_actin_data_external_latest_uri/\*" "$shared_actin_data_external_uri/"
+fi
+
+info "Copying ACTIN data for ${patient} to ${shared_actin_data_external_latest_uri}"
+gsutil cp "${patient_record_json}" "${shared_actin_data_external_latest_uri}/"
+gsutil cp "${treatment_match_json}" "${shared_actin_data_external_latest_uri}/"
+gsutil cp "${actin_report_pdf}" "${shared_actin_data_external_latest_uri}/"


Do we need some checks to handle if one of these operations fails? We wouldn't want to overwrite files if the archiving step is unsuccessful.

Probably that is best. I will add.

jbartletthmf · 2025-07-24T14:12:44Z

actin/local/operational/share_actin_data_and_report_with_nki

+if [[ "$version_folder" != "1" ]]; then
+  info "Moving ACTIN data from $shared_actin_data_external_latest_uri to $shared_actin_data_external_uri"
+  gsutil -m mv "$shared_actin_data_external_latest_uri/\*" "$shared_actin_data_external_uri/"
+fi


I'm not sure how practical this is, but it would be nice if this code could be centralized and reused instead of implemented in each file. It would be even better if we had a well-tested application to manage files and sharing, but now I'm just being unreasonable 😛

I can try to generalize it so I can create a reusable function.

jbartletthmf · 2025-07-24T14:16:53Z

actin/local/operational/share_actin_data_and_report_with_emc

+version_folder=$(basename "${shared_actin_data_external_uri%/}")
+shared_actin_data_external_latest_uri="${shared_actin_data_external_uri/%${version_folder}/}latest"
+
+if [[ "$version_folder" != "1" ]]; then


So nothing is moved when the folder equals 1? Does that mean that the first archived version will be 2? Should we subtract 1 so the archives start from 1 instead?

If the folder is 1 it means it's the first version and there is no existing latest.

Right, and in that case, we skip archiving and just copy the new files to latest. On the next run, there still won't be a folder called 1, so will we try with version_folder 1 again? It doesn't look like the version management is aware of latest/, which would mean nothing gets archived.

Alternatively, if the version gets bumped to 2, it looks like the first archive operation will be to copy from latest/ to 2/.

andreia-hartwig · 2025-07-24T14:37:29Z

It's unfortunate that Google storage doesn't support symlinks or atomic move operations, because having to move the latest folder every time there's an update isn't ideal. Given those constraints, I don't see a better way to have a constant path for the latest data. Some alternatives would be to:

store the latest version in a separate file at the top level of the directory, so consumers of the data could read that file first to determine the path

resolve the latest path at access time by listing the directories

Both of those options simplify updates but complicate reads. Since we update in one place and may read from many, it probably makes sense to stick with the current approach.

I also don't fully like the idea of move the latest folder every time but adding the latest folder was a request from Jana to simplify reading the data. I think this could also be useful at UMCU, for example, so I was trying to put the effort on the writing side since we only do that once. I even considered always creating a versioned folder and duplicating the files in a latest folder, but since most patients will only have one version, that felt like overkill and a waste of resources.

jbartletthmf · 2025-07-24T14:45:44Z

Another alternative would be to have a URL that resolves to the latest version, but I don't know if that would support all the existing (and future) use cases.

…us latest to a versioned folder." This reverts commit 40820cd.

andreia-hartwig · 2025-07-25T09:53:57Z

@jbartletthmf Can you please review the latest commit? I've added a manifest file to help identify the latest version.

jbartletthmf

Much tidier! Nicely done 🙂

andreia-hartwig and others added 3 commits July 20, 2025 14:33

ACTIN-2302 add comments

bd7d317

Merge branch 'master' into ACTIN-2312

876b486

andreia-hartwig requested review from ninajacobs and jbartletthmf July 21, 2025 11:51

andreia-hartwig mentioned this pull request Jul 21, 2025

ACTIN-2313 remove actin-evaluation code as it's no longer needed hartwigmedical/actin#1266

Open

ninajacobs reviewed Jul 21, 2025

View reviewed changes

jbartletthmf approved these changes Jul 21, 2025

View reviewed changes

ACTIN-2312 renaming scripts

37db9a8

andreia-hartwig requested a review from ninajacobs July 21, 2025 15:53

ninajacobs approved these changes Jul 21, 2025

View reviewed changes

andreia-hartwig added 2 commits July 21, 2025 18:37

ACTIN-2312 renaming

75d3453

ACTIN-2312 add ability to skip copying the orange data

7f8055f

jbartletthmf approved these changes Jul 22, 2025

View reviewed changes

ACTIN-2312 add new files to a latest folder moving the previous lates…

40820cd

…t to a versioned folder.

andreia-hartwig requested review from ninajacobs and jbartletthmf July 24, 2025 13:18

jbartletthmf reviewed Jul 24, 2025

View reviewed changes

andreia-hartwig added 2 commits July 25, 2025 11:00

Revert "ACTIN-2312 add new files to a latest folder moving the previo…

3627b10

…us latest to a versioned folder." This reverts commit 40820cd.

ACTIN-2312 add manifest json to identify latest version of the data

b886568

andreia-hartwig requested a review from jbartletthmf July 25, 2025 09:53

jbartletthmf approved these changes Jul 25, 2025

View reviewed changes

		@@ -38,7 +38,7 @@ if [[ -z "${orange_no_germline_json}" && -z "${orange_no_germline_pdf}" ]]; then
		warn "Skipping archiving of ORANGE data for ${patient} as they do not exist"

ACTIN-2312 Align shared data buckets between EMC and NKI #112

Are you sure you want to change the base?

ACTIN-2312 Align shared data buckets between EMC and NKI #112

Uh oh!

Conversation

andreia-hartwig commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreia-hartwig Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ninajacobs Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreia-hartwig Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreia-hartwig commented Jul 24, 2025

Uh oh!

jbartletthmf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreia-hartwig commented Jul 24, 2025

Uh oh!

jbartletthmf commented Jul 24, 2025

Uh oh!

andreia-hartwig commented Jul 25, 2025

Uh oh!

jbartletthmf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andreia-hartwig commented Jul 21, 2025 •

edited

Loading

andreia-hartwig Jul 21, 2025 •

edited

Loading

ninajacobs Jul 21, 2025 •

edited

Loading

andreia-hartwig Jul 21, 2025 •

edited

Loading