-
Notifications
You must be signed in to change notification settings - Fork 4
Murko: continue thaw if redis is down #1437
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
Open
jacob720
wants to merge
14
commits into
main
Choose a base branch
from
810_continue_thaw_if_redis_is_down
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 9 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
4cfe0cc
Check redis connection and thaw if connection fails
jacob720 779870c
Clean up tests
jacob720 bfd0963
Small change
jacob720 dfd1cb9
Add test
jacob720 37fda8c
Move connection check into murko_callback
jacob720 15d5a5c
Fix
jacob720 641bbbc
Fix and add tests
jacob720 ea92171
Move warning into function
jacob720 ffc19e8
Pin dodal
jacob720 d01688d
Merge branch 'main' into 810_continue_thaw_if_redis_is_down
jacob720 82f35a3
Check connection in start rather than callback init
jacob720 6bb52f1
Improve test
jacob720 f75054e
Add test
jacob720 64261f8
Update dls-dodal dependency to use main branch
DominicOram File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -263,7 +263,9 @@ def test_thaw_and_stream_adds_murko_callback_and_produces_expected_messages( | |
|
|
||
| @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback.stop") | ||
| @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback.call_murko") | ||
| @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback._check_redis_connection") | ||
| def test_thaw_and_stream_will_produce_events_that_call_murko( | ||
| patch_check_redis_connection: MagicMock, | ||
| patch_murko_call: MagicMock, | ||
| patch_stop_call: MagicMock, | ||
| smargon: Smargon, | ||
|
|
@@ -272,6 +274,8 @@ def test_thaw_and_stream_will_produce_events_that_call_murko( | |
| oav_forwarder: OAVToRedisForwarder, | ||
| run_engine: RunEngine, | ||
| ): | ||
| patch_check_redis_connection.return_value = True | ||
|
|
||
| class StopPlanError(Exception): | ||
| pass | ||
|
|
||
|
|
@@ -367,7 +371,7 @@ def _run_thaw_and_stream_and_assert_zoom_changes( | |
|
|
||
| @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback") | ||
| def test_given_thaw_succeeds_then_thaw_and_stream_sets_zoom_to_1_and_back( | ||
| patch_murko_callback, | ||
| patch_murko_callback: MagicMock, | ||
| smargon: Smargon, | ||
| thawer: Thawer, | ||
| oav_forwarder: OAVToRedisForwarder, | ||
|
|
@@ -383,8 +387,8 @@ def test_given_thaw_succeeds_then_thaw_and_stream_sets_zoom_to_1_and_back( | |
| @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback") | ||
| @patch("mx_bluesky.beamlines.i04.thawing_plan.bps.monitor") | ||
| def test_given_thaw_fails_then_thaw_and_stream_sets_zoom_to_1_and_back( | ||
| mock__thaw, | ||
| patch_murko_callback, | ||
| mock__thaw: MagicMock, | ||
| patch_murko_callback: MagicMock, | ||
| smargon: Smargon, | ||
| thawer: Thawer, | ||
| oav_forwarder: OAVToRedisForwarder, | ||
|
|
@@ -403,8 +407,8 @@ def test_given_thaw_fails_then_thaw_and_stream_sets_zoom_to_1_and_back( | |
| "mx_bluesky.beamlines.i04.thawing_plan._rotate_in_one_direction_and_stream_to_redis" | ||
| ) | ||
| def test_thaw_and_murko_centre_stages_and_unstages_murko_results_twice( | ||
| mock_rotate_and_stream, | ||
| patch_murko_callback, | ||
| mock_rotate_and_stream: MagicMock, | ||
| patch_murko_callback: MagicMock, | ||
| smargon: Smargon, | ||
| thawer: Thawer, | ||
| oav_forwarder: OAVToRedisForwarder, | ||
|
|
@@ -424,8 +428,8 @@ def test_thaw_and_murko_centre_stages_and_unstages_murko_results_twice( | |
| @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback") | ||
| @patch("mx_bluesky.beamlines.i04.thawing_plan.bps.monitor") | ||
| def test_given_thaw_and_murko_centre_errors_then_murko_results_still_unstaged( | ||
| mock__thaw, | ||
| patch_murko_callback, | ||
| mock__thaw: MagicMock, | ||
| patch_murko_callback: MagicMock, | ||
| smargon: Smargon, | ||
| thawer: Thawer, | ||
| oav_forwarder: OAVToRedisForwarder, | ||
|
|
@@ -583,3 +587,44 @@ def test_thawing_plan_with_murko_callback_puts_correct_metadata_into_redis( | |
| assert publish_call_args_list[1].args[1] == json.dumps(FORWARDING_COMPLETE_MESSAGE) | ||
| assert publish_call_args_list[2].args[1] == json.dumps(expected_roi_md) | ||
| assert publish_call_args_list[3].args[1] == json.dumps(FORWARDING_COMPLETE_MESSAGE) | ||
|
|
||
|
|
||
| @patch( | ||
| "mx_bluesky.beamlines.i04.thawing_plan._rotate_in_one_direction_and_stream_to_redis" | ||
| ) | ||
| @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback._check_redis_connection") | ||
| def test_plans_carry_on_thaw_if_redis_connection_check_fails( | ||
| patch_callback_check_redis_connection: MagicMock, | ||
| patch_rotate_in_one_direction_and_stream_to_redis: MagicMock, | ||
| smargon: Smargon, | ||
| thawer: Thawer, | ||
| robot: BartRobot, | ||
| oav_forwarder: OAVToRedisForwarder, | ||
| run_engine: RunEngine, | ||
| ): | ||
| patch_callback_check_redis_connection.return_value = False | ||
| murko_results = MurkoResultsDevice() | ||
| murko_results._check_redis_connection = AsyncMock(return_value=False) | ||
| for plan in ( | ||
| thaw_and_murko_centre( | ||
| 10, | ||
| 360, | ||
| thawer=thawer, | ||
| smargon=smargon, | ||
| robot=robot, | ||
| oav_to_redis_forwarder=oav_forwarder, | ||
| murko_results=murko_results, | ||
| ), | ||
| thaw_and_stream_to_redis( | ||
| 10, | ||
| 360, | ||
| thawer=thawer, | ||
| smargon=smargon, | ||
| robot=robot, | ||
| oav_to_redis_forwarder=oav_forwarder, | ||
| ), | ||
| ): | ||
| run_engine(plan) | ||
|
|
||
| assert patch_rotate_in_one_direction_and_stream_to_redis.call_count == 2 | ||
| patch_rotate_in_one_direction_and_stream_to_redis.reset_mock() | ||
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Could: It might be better to re-check the connection on
start. For other callbacks we only create them once when we start up and never recreate them. I know we currently don't forMurkoCallbackbut if we did want to this would mean that we would have to restart the whole env to pick up redis coming back up