-
Notifications
You must be signed in to change notification settings - Fork 0
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
Refactor Python and Orthanc Dockerfiles #583
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #583 +/- ##
==========================================
- Coverage 87.38% 85.17% -2.22%
==========================================
Files 76 72 -4
Lines 3386 3116 -270
==========================================
- Hits 2959 2654 -305
- Misses 427 462 +35 ☔ View full report in Codecov by Sentry. |
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.
looking really nice - just a question around downloading the DICOM spec for Orthanc Anon
COPY ./orthanc/orthanc-anon/plugin/download_dicom_spec.py /etc/orthanc/download_dicom_spec.py | ||
RUN --mount=type=cache,target=/root/.cache \ | ||
python3 /etc/orthanc/download_dicom_spec.py | ||
|
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.
downloading the Python spec is pretty slow to do, so I think ideally this would be one of the earlier layers in the image. Not sure the best way to do that? Either include it in Raw, or have separate Raw and Anon images?
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.
Looks good, will leave it to you and Paul to work out the best way to have the python spec downloaded and cached
Description
Fixes #{issue_number}:
A cut down version of #400 , with just the Dockerfile de-duping. If we decide #400 is too risky to merge now, we can at least merge this one and do the rest of #400 later. The advantage is it enables further work on the Dockerfiles, such as switching to
uv
.Type of change
Please delete options accordingly to the description.
Suggested Checklist
main
branch.squash and merge