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

Manage storage size on orthanc-raw #176

Closed
wants to merge 4 commits into from
Closed

Manage storage size on orthanc-raw #176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0842f43
Fix env variable names in `delete_oldest_n_studies.py`
milanmlft Dec 6, 2023
9e11816
Add MaximumStorageSize config for orthanc-raw
milanmlft Dec 6, 2023
1cdf7f0
MaximumStorageSize needs to be an integer
milanmlft Dec 6, 2023
55972d5
Revert "Fix env variable names in `delete_oldest_n_studies.py`"
milanmlft Dec 7, 2023
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
1 change: 1 addition & 0 deletions .env.sample
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ ORTHANC_RAW_USERNAME=
ORTHANC_RAW_PASSWORD=
ORTHANC_RAW_AE_TITLE=
ORTHANC_AUTOROUTE_RAW_TO_ANON=true
ORTHANC_RAW_MAXIMUM_STORAGE_SIZE=

# PIXL Orthanc anon instance
ORTHANC_ANON_USERNAME=
Expand Down
1 change: 1 addition & 0 deletions docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ services:
ORTHANC_PASSWORD: ${ORTHANC_RAW_PASSWORD}
ORTHANC_RAW_AE_TITLE: ${ORTHANC_RAW_AE_TITLE}
ORTHANC_AUTOROUTE_RAW_TO_ANON: ${ORTHANC_AUTOROUTE_RAW_TO_ANON}
ORTHANC_RAW_MAXIMUM_STORAGE_SIZE: ${ORTHANC_RAW_MAXIMUM_STORAGE_SIZE}
VNA_AE_TITLE : ${VNA_AE_TITLE}
VNA_DICOM_PORT: ${VNA_DICOM_PORT}
VNA_IP_ADDR: ${VNA_IP_ADDR}
Expand Down
7 changes: 5 additions & 2 deletions orthanc/orthanc-raw/config/orthanc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@
// doubling them, or replaced by forward slashes "/".
"StorageDirectory" : "/var/lib/orthanc/db",

// Limit the maximum number of instances
"MaximumPatientCount": 20000,
// Limit the maximum storage size
"MaximumPatientCount" : 0, // no limit
"MaximumStorageSize" : ${ORTHANC_RAW_MAXIMUM_STORAGE_SIZE}, // MB
Copy link
Member Author

@milanmlft milanmlft Dec 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Turns out this doesn't work and results in an error ERROR: Unable to parse Json file '/run/secrets/orthanc.json'; check syntax , with the orthanc-raw service not even starting.

Using

  "MaximumStorageSize" : "${ORTHANC_RAW_MAXIMUM_STORAGE_SIZE}", // MB

in turn leads to the MaximumStorageSize parameter being ignored, because of a Bad parameter type error, as it needs to be an integer...

I've tried a couple of different things but it seems it's not possible get an environment variable read in as an integer in orthanc.json 🙁

@stefpiatek, @t-young31 any ideas?

Copy link
Member

Choose a reason for hiding this comment

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

😞
no good ones. could: (a) hardcode, (b) copy the config file into the image and sed from a build argument. I don't suppose it works if you remove the quotes?

Copy link
Contributor

@stefpiatek stefpiatek Dec 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah I would have thought having the env variable without the quotes would work, sad if it doesn't. Agreed on using sed to replace inline if not, perhaps wrapping the variable usage with something that will make that replacement unique

"MaximumStorageMode" : "Recycle",
Comment on lines +13 to +16
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉 Yeah this seems like a good shout. Worth testing on a dev version on the GAE. Which also reminds me that we need to document how we create a shared instance on the GAE when we make them

I'll make a PR based on step 1 of https://github.com/UCLH-DHCT/emap/edit/main/docs/core.md



// Path to the directory that holds the SQLite index (if unset, the
// value of StorageDirectory is used). This index could be stored on
Expand Down
1 change: 1 addition & 0 deletions test/.env.test.sample
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ ORTHANC_RAW_USERNAME=orthanc_raw_username
ORTHANC_RAW_PASSWORD=orthanc_raw_password
ORTHANC_RAW_AE_TITLE=ORTHANCRAW
ORTHANC_AUTOROUTE_RAW_TO_ANON=true
ORTHANC_RAW_MAXIMUM_STORAGE_SIZE=1
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want this to be limited during testing?

Copy link
Member Author

@milanmlft milanmlft Dec 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think having a low value makes testing easier as we won't need a huge amount of images to see if it's actually working. Note that this is just the sample file though, the actual .env.test is ignored so can easily be adapted locally. The 1 (MB) is just a default value for now.

Copy link
Contributor

@stefpiatek stefpiatek Dec 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Ah yeah I guess if this is used in end to end tests then that might be an issue was my thought, but maybe it's not used in e2e tests in CI?


# PIXL Orthanc anon instance
ORTHANC_ANON_USERNAME=orthanc_anon_username
Expand Down