[uss_qualifier] subscription_interactions: verify secondary DSS instances are clean

[Shastick]

This PR is a step in service of #810. It also introduces new Exceptions so as to follow what is outlined in #1130

[the-glu]

Using assert outside of (unit) tests is not a good idea: https://docs.astral.sh/ruff/rules/assert/

[Shastick]

Ah shoot. I guess we can roll our own function that would raise something like an "UnexectedStateError" or similar. I'll quickly look at what I can come up with

[Shastick]

Questions, though:

• do we use -0?
• this is mostly about conveying information to the linter (currently, if the assert statement would not exist we'd have an exception raised anyway)

assert could be an option out here, in my opinion

[the-glu]

• We can't really know, especially since anyone could run the code how they want ^^'
• I would still raise it explicitly, it's better than implicitly :)

[BenjaminPelletier]

I'm not sure of the value of this method. We don't want to do standard flow control through exceptions, so it seems like the purpose of this method could be achieved by retrieving the normal subscription property and checking if it was None or not. Even if we did want separate methods, we want to avoid duplication as much as practical, so it seems like the implementation would be something like:

@property
def subscription_unsafe(self) -> Subscription:
    result = self.subscription
    if result is None:
        raise ValueError("Request to get Subscription did not return valid JSON")

[BenjaminPelletier]

Why use the unsafe version here? It seems like that invites an exception we're not prepared to handle. I think assuring subscription isn't None could be cleanly incorporated into the check above:

    with scenario.check(
        "Subscription can be queried by ID", [dss.participant_id]
    ) as check:
        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.error_message}",
                query_timestamps=[existing_sub.request.timestamp],
            )
        sub_to_delete = None
        if existing_sub.status_code == 200:
            sub_to_delete = existing_sub.subscription
            if sub_to_delete is None:
                check.record_failed(...)

    if not delete_if_exists or not sub_to_delete:
        return False

    deleted_sub = dss.delete_subscription(sub_id, sub_to_delete.version

[Shastick]

Explicitly making all required checks seems like the way to go. If we do this I don't think we need something like subscription_unsafe.

[BenjaminPelletier]

This looks good, but we should put more information in the default message. At a minimum, something like "A variable was None when it should have been impossible for that variable to be None". We should strive for a developer finding an error message to instantly understand the nature of the problem and have the information they need to address it.

[Shastick]

I'll mix in the suggestion you added on #1130, which should provide enough details.

[BenjaminPelletier]

nit: since the participant string will be, e.g., "uss1, uss2", this will produce a string like

check 'Operational intent references can be queried by ID'uss1, uss2 (severity High https://github.com/interuss/monitoring/blob/main/monitoring/uss_qualifier/scenarios/astm/utm/dss/fragments/oir/oir_has_expected_subscription.md#-oir-is-attached-to-expected-subscription-check)

...which doesn't seem super clear or clean.

Suggested change

	return f"check '{doc.name}'{participant_str} (severity {severity}{url_str})"
	return f"'{doc.name} check' ({severity} severity involving {participant_str}) documented at {url_str})"

...which will produce

'Operational intent references can be queried by ID check' (High severity involving uss1, uss2) documented at https://github.com/interuss/monitoring/blob/main/monitoring/uss_qualifier/scenarios/astm/utm/dss/fragments/oir/oir_has_expected_subscription.md#-oir-is-attached-to-expected-subscription-check

This can be adjusted in a separate PR.