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.

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

Process large studies and prod fixes #416

Merged
merged 37 commits into from
Jul 17, 2024
Merged

Process large studies and prod fixes #416

merged 37 commits into from
Jul 17, 2024

Conversation

stefpiatek
Copy link
Contributor

@stefpiatek stefpiatek commented Jul 2, 2024

Had a whole lot of fun with running in production:

  • Tweaking of logging
  • I think if the GAE has a lot of CPU use then the resource modification can freeze up, this decreases if we don't limit the number of threads that can be used for this so
  • Tried cancelling jobs on failure, but it doesn't actually cancel them so removed
  • Actually use tqdm properly 🤦
  • Use DICOM transfer timeout across services in case of large studies
  • Increase concurrent jobs in orthanc anon as we have done in orthanc raw
  • Make the stable age in orthanc raw 60 seconds longer than the timeout, we were deleting studies that hung after 60 seconds and then we'd get more data and export the latter half of a study!
  • Use http timeout for export api, get the status back of the export in case it fails - could make an issue to convert this to async job and check that job status?
  • Delete a study if it is stable but has not had the default pixl project tag overriden in orthanc raw, as a way to gate exit of closes Avoid unknown projects being sent to orthanc-anon #395
  • Be less busy in polling for job status in imaging api, every 10 seconds is fine

TODO:

  • Document changes from the end users perspective in README
  • Document suggested values for configuration and alter the defaults to match production
  • Increase test coverage?

@stefpiatek stefpiatek requested a review from milanmlft July 2, 2024 10:18
Copy link

codecov bot commented Jul 2, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 9 lines in your changes missing coverage. Please review.

Project coverage is 82.15%. Comparing base (cbf9e3b) to head (ea84639).

Files Patch % Lines
pixl_imaging/src/pixl_imaging/_orthanc.py 61.11% 7 Missing ⚠️
pixl_imaging/src/pixl_imaging/_processing.py 77.77% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #416      +/-   ##
==========================================
- Coverage   83.54%   82.15%   -1.39%     
==========================================
  Files          82       79       -3     
  Lines        3445     3228     -217     
==========================================
- Hits         2878     2652     -226     
- Misses        567      576       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@milanmlft milanmlft left a comment

Choose a reason for hiding this comment

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

Looking great! Though could indeed use a bit more test coverage

docker-compose.yml Show resolved Hide resolved
orthanc/orthanc-raw/README.md Outdated Show resolved Hide resolved
orthanc/orthanc-raw/plugin/pixl.py Outdated Show resolved Hide resolved
@stefpiatek stefpiatek enabled auto-merge (squash) July 17, 2024 16:55
@stefpiatek stefpiatek merged commit cfa2307 into main Jul 17, 2024
10 checks passed
@stefpiatek stefpiatek deleted the ms-pinpoint-export branch July 17, 2024 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid unknown projects being sent to orthanc-anon
2 participants