From 0caf523f5c3e9cf81de9a38b229b2f06884156ed Mon Sep 17 00:00:00 2001 From: Julien Perrochet Date: Wed, 13 Mar 2024 18:05:33 +0100 Subject: [PATCH] PR comments --- .../subscription_synchronization.md | 10 +--- .../subscription_synchronization.py | 54 ++++++++++--------- .../astm/utm/dss/test_step_fragments.py | 2 +- 3 files changed, 32 insertions(+), 34 deletions(-) diff --git a/monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.md b/monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.md index 4e44825feb..0e25cd78a7 100644 --- a/monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.md +++ b/monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.md @@ -156,7 +156,7 @@ Verify that the version of the subscription returned by the DSS is as expected Attempt to query and search for the deleted subscription in various ways -#### 🛑 Secondary DSS should not return the deleted subscription check +#### 🛑 DSS should not return the deleted subscription check If a DSS returns a subscription that was previously successfully deleted from the primary DSS, either one of the primary DSS or the DSS that returned the subscription is in violation of **[astm.f3548.v21.DSS0210,1a](../../../../../requirements/astm/f3548/v21.md)**. @@ -177,15 +177,9 @@ Verify that the subscription returned by the DSS via the deletion is properly fo Verify that the version of the subscription returned by the DSS is as expected -#### 🛑 Secondary DSS should not return the deleted subscription check +#### 🛑 DSS should not return the deleted subscription check If a DSS returns a subscription that was previously successfully deleted from the primary DSS, either one of the primary DSS or the DSS that returned the subscription is in violation of **[astm.f3548.v21.DSS0210,1a](../../../../../requirements/astm/f3548/v21.md)**. -#### 🛑 Primary DSS should not return the deleted subscription check - -If the primary DSS returns a subscription that was previously successfully deleted from a secondary DSS, -either one of the secondary or primary DSS is in violation of **[astm.f3548.v21.DSS0210,1a](../../../../../requirements/astm/f3548/v21.md)**. - - ## [Cleanup](../clean_workspace.md) diff --git a/monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.py b/monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.py index 5b1b31e854..c134ecf38f 100644 --- a/monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.py +++ b/monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.py @@ -220,8 +220,6 @@ def _ensure_no_active_subs_exist(self): def _step_create_subscriptions(self): # Create the 'main' test subscription: main_sub = self._create_sub_with_params(self._sub_params) - if main_sub is None: - return self._current_subscription = main_sub @@ -230,14 +228,12 @@ def _step_create_subscriptions(self): params = self._sub_params.copy() params.sub_id = sub_id extra_sub = self._create_sub_with_params(params) - if extra_sub is None: - return self._subs_for_deletion[sub_id] = extra_sub self._subs_for_deletion_params[sub_id] = params def _create_sub_with_params( self, creation_params: SubscriptionParams - ) -> Optional[Subscription]: + ) -> Subscription: # TODO migrate to the try/except pattern for queries newly_created = self._dss.upsert_subscription( @@ -255,7 +251,6 @@ def _create_sub_with_params( details=f"Subscription creation failed with status code {newly_created.status_code}", query_timestamps=[newly_created.request.timestamp], ) - return None with self.check( "Create subscription response is correct", [self._primary_pid] @@ -556,7 +551,7 @@ def _compare_upsert_resp_with_params( def _mutate_subscription_with_dss( self, dss_instance: DSSInstance, new_params: SubscriptionParams - ) -> bool: + ): """ Mutate the subscription on the given DSS instance using the given parameters. Also updates the internal state of the scenario to reflect the new subscription. @@ -574,7 +569,6 @@ def _mutate_subscription_with_dss( details=f"Subscription mutation failed with status code {mutated_sub_response.status_code}", query_timestamps=[mutated_sub_response.request.timestamp], ) - return False # Check that what we get back is valid and corresponds to what we want to create self._compare_upsert_resp_with_params( @@ -584,7 +578,6 @@ def _mutate_subscription_with_dss( self._current_subscription = mutated_sub_response.subscription # Update the parameters we used for that subscription self._sub_params = new_params - return True def _step_mutate_subscriptions_broadcast_shift_time(self): """Mutate the subscription on the primary DSS by adding 10 seconds to its start and end times""" @@ -599,12 +592,10 @@ def _step_mutate_subscriptions_secondaries_shift_time(self): for secondary_dss in self._dss_read_instances: # Mutate the subscription on the secondary DSS - if not self._mutate_subscription_with_dss( + self._mutate_subscription_with_dss( secondary_dss, shift_time_params(self._sub_params, timedelta(seconds=10)), - ): - # If the mutation failed but the scenario has not terminated, we end this step here. - return + ) # Check that the mutation was propagated to every DSS: self._query_secondaries_and_compare(self._sub_params) @@ -674,29 +665,42 @@ def _step_delete_subscriptions_on_secondaries(self): # If the deletion failed but the scenario has not terminated, we end this step here. return # Check that the primary knows about the deletion: - self._confirm_dss_has_no_sub(self._dss, sub_id, is_primary=True) + self._confirm_dss_has_no_sub( + self._dss, sub_id, secondary_dss.participant_id + ) # Check that the deletion was propagated to every DSS: - self._confirm_no_secondary_has_sub(sub_id) + self._confirm_no_secondary_has_sub(sub_id, secondary_dss.participant_id) def _step_get_deleted_sub(self): - self._confirm_no_secondary_has_sub(self._sub_id) + self._confirm_no_secondary_has_sub(self._sub_id, self._dss.participant_id) - def _confirm_no_secondary_has_sub(self, sub_id: str): + def _confirm_no_secondary_has_sub( + self, sub_id: str, deleted_on_participant_id: str + ): + """Confirm that no secondary DSS has the subscription. + deleted_on_participant_id specifies the participant_id of the DSS where the subscription was deleted.""" for secondary_dss in self._dss_read_instances: - self._confirm_dss_has_no_sub(secondary_dss, sub_id, is_primary=False) + self._confirm_dss_has_no_sub( + secondary_dss, sub_id, deleted_on_participant_id + ) def _confirm_dss_has_no_sub( - self, dss_instance: DSSInstance, sub_id: str, is_primary: bool = False + self, + dss_instance: DSSInstance, + sub_id: str, + other_participant_id: Optional[str], ): - check_name = ( - "Primary DSS should not return the deleted subscription" - if is_primary - else "Secondary DSS should not return the deleted subscription" + """Confirm that a DSS has no subscription. + other_participant_id may be specified if a failed check may be caused by it.""" + participants = ( + [dss_instance.participant_id, other_participant_id] + if other_participant_id + else None ) fetched_sub = dss_instance.get_subscription(sub_id) with self.check( - check_name, - [dss_instance.participant_id], + "DSS should not return the deleted subscription", + participants, ) as check: if fetched_sub.status_code != 404: check.record_failed( diff --git a/monitoring/uss_qualifier/scenarios/astm/utm/dss/test_step_fragments.py b/monitoring/uss_qualifier/scenarios/astm/utm/dss/test_step_fragments.py index 228ce18089..fab8fe60ba 100644 --- a/monitoring/uss_qualifier/scenarios/astm/utm/dss/test_step_fragments.py +++ b/monitoring/uss_qualifier/scenarios/astm/utm/dss/test_step_fragments.py @@ -51,7 +51,7 @@ def cleanup_sub( if existing_sub.status_code not in [200, 404]: check.record_failed( summary=f"Could not query subscription {sub_id}", - details=f"When attempting to query subscription {sub_id} from the DSS, received {existing_sub.status_code}: {existing_sub.json_result}", + details=f"When attempting to query subscription {sub_id} from the DSS, received {existing_sub.status_code}: {existing_sub.error_message}", query_timestamps=[existing_sub.request.timestamp], )