Add a separate queue for querying PACS #503

p-j-smith · 2024-09-30T08:15:03Z

Description

Fixes https://github.com/SAFEHR-data/the-rolling-skeleton/issues/416

add a second queue (_imaging_secondary) for querying PACS
messages processed by the existing imaging queue will only query the VNA. If a study is not available in the VNA, the corresponding message will be published to the new secondary imaging queue
messages are only processed by the secondary imaging queue outside of working hours (as we don't want to query PACS during working hours)

Type of change

Please delete options accordingly to the description.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Suggested Checklist

I have performed a self-review of my own code.
I have made corresponding changes to the documentation.
My changes generate no new warnings.
I have commented my code, particularly in hard-to-understand areas.
I have passed on my local host device. (see further details at the CONTRIBUTING document)
Make sure your branch is up-to-date with main branch. See CONTRIBUTING for a general example to syncronise your branch with the main branch.
I have requested PR review to UCLH-Foundtry/arc-dev
I have adddressed and marked as resolved all the review comments in my PR.
Finally, I have selected squash and merge

Also add a PixlOutOfHoursError that is raised by the pacs queue when out of hours

…PrimaryArchiveError error The consumer then sends the message to the secondary imaging queue. Only process messages in the secondary imaging queue outside of working hours. During working hours, re-queue the message

codecov · 2024-09-30T08:16:26Z

Codecov Report

Attention: Patch coverage is 66.19718% with 24 lines in your changes missing coverage. Please review.

Project coverage is 87.31%. Comparing base (c7f42c7) to head (59bc5c3).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pixl_core/src/core/patient_queue/subscriber.py	28.57%	10 Missing ⚠️
pixl_imaging/src/pixl_imaging/main.py	0.00%	8 Missing ⚠️
cli/src/pixl_cli/_message_processing.py	20.00%	4 Missing ⚠️
pixl_imaging/src/pixl_imaging/_processing.py	92.85%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #503      +/-   ##
==========================================
- Coverage   87.78%   87.31%   -0.48%     
==========================================
  Files          76       76              
  Lines        3415     3438      +23     
==========================================
+ Hits         2998     3002       +4     
- Misses        417      436      +19

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

stefpiatek

Some of this may be outdated because I thought I submitted 🤦🏻

pixl_core/src/core/patient_queue/subscriber.py

pixl_core/src/core/token_buffer/tokens.py

pixl_imaging/src/pixl_imaging/_processing.py

pixl_imaging/src/pixl_imaging/main.py

stefpiatek · 2024-10-01T09:30:31Z

Oh also its worth checking the checklist in https://github.com/SAFEHR-data/the-rolling-skeleton/issues/416 to make sure that we've covered all of that

…wo imaging api queues

…aging-primary

…cuase the study is not in the VNA

stefpiatek

Looking good, would be nice to do set off a run overnight tonight to check that it successfuly runs through both queues still (no need to take volumes down)

stefpiatek · 2024-10-02T15:18:40Z

cli/src/pixl_cli/_config.py

+        "imaging-primary": "imaging_api",
+        "imaging-secondary": "imaging_api",


oooh this is nice

stefpiatek · 2024-10-02T15:20:02Z

cli/src/pixl_cli/_message_processing.py

+    # We don't want to modify the queues we're populating, but if we're populating imaging-primary
+    # we also need to wait for imaging-secondary to be empty
+    queues_to_count = queues_to_populate.copy()
+    if "imaging-primary" in queues_to_populate and "imaging-secondary" not in queues_to_populate:
+        queues_to_count.append("imaging-secondary")


could make just make this a set and then add it to it without checking?

p-j-smith · 2024-10-02T16:00:44Z

Looking good, would be nice to do set off a run overnight tonight to check that it successfuly runs through both queues still (no need to take volumes down)

yeah good shout - I've set it running on pixl_test

p-j-smith · 2024-10-07T11:19:45Z

Patch coverage is a bit low but it's mostly on hard to test areas (we're mocking a lot of these parts of the code during tests), so hopefully it's okay to merge as is

p-j-smith added 3 commits September 27, 2024 10:22

use 'primary' and 'secondary' keys for the token bucket

dd9a5b7

Add a secondary imaging queue for querying pacs

94a1bab

Also add a PixlOutOfHoursError that is raised by the pacs queue when out of hours

p-j-smith requested a review from a team September 30, 2024 08:15

Add key to call to TokenBucket.has_token in tests

fbac1ac

stefpiatek approved these changes Sep 30, 2024

View reviewed changes

p-j-smith added 8 commits October 2, 2024 09:08

Update imaging api docs to describe the processing performed by the t…

4742b79

…wo imaging api queues

rename imaging queues to imaging-primary and imaging-secondary

8f08d63

Also wait for imaging-secondary queue to empty if we're populating im…

6a15f29

…aging-primary

Use info level logging when rejecting message from imaging-primary be…

33a9514

…cuase the study is not in the VNA

Also log the archive we're querying when processing a message

2f27126

Fix mapping of queue names to api names in cli._config

6531e4f

Add test for invalid token key

40c725d

Merge branch 'main' into paul/separate-queue-for-pacs

f7e9d9c

stefpiatek approved these changes Oct 2, 2024

View reviewed changes

Make queues_to_count a set rather than a list

8984709

p-j-smith added 2 commits October 3, 2024 10:14

rename 'imaging' queue to 'imaging-primary'

db389f4

Merge branch 'main' into paul/separate-queue-for-pacs

59bc5c3

p-j-smith merged commit 137badd into main Oct 7, 2024
10 of 11 checks passed

p-j-smith deleted the paul/separate-queue-for-pacs branch October 7, 2024 11:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a separate queue for querying PACS #503

Add a separate queue for querying PACS #503

p-j-smith commented Sep 30, 2024 •

edited

Loading

codecov bot commented Sep 30, 2024 •

edited

Loading

stefpiatek left a comment

stefpiatek commented Oct 1, 2024

stefpiatek left a comment

stefpiatek Oct 2, 2024

stefpiatek Oct 2, 2024

p-j-smith commented Oct 2, 2024

p-j-smith commented Oct 7, 2024

		"imaging-primary": "imaging_api",
		"imaging-secondary": "imaging_api",

Add a separate queue for querying PACS #503

Add a separate queue for querying PACS #503

Conversation

p-j-smith commented Sep 30, 2024 • edited Loading

Description

Type of change

Suggested Checklist

codecov bot commented Sep 30, 2024 • edited Loading

Codecov Report

stefpiatek left a comment

Choose a reason for hiding this comment

stefpiatek commented Oct 1, 2024

stefpiatek left a comment

Choose a reason for hiding this comment

stefpiatek Oct 2, 2024

Choose a reason for hiding this comment

stefpiatek Oct 2, 2024

Choose a reason for hiding this comment

p-j-smith commented Oct 2, 2024

p-j-smith commented Oct 7, 2024

p-j-smith commented Sep 30, 2024 •

edited

Loading

codecov bot commented Sep 30, 2024 •

edited

Loading