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

[uss_qualifier] check that implicit SCD subscriptions are properly managed #720

Merged

Conversation

Shastick
Copy link
Contributor

@Shastick Shastick commented Jun 17, 2024

PR for a scenario addressing implicit subscription handling, based on checks proposed in #415.

This depends on this DSS fix to be merged and released. The PR was tested locally against a fixed DSS and the scenario passes.

Some improvements that we could add in later PRs

Misc

  • Adds a force_no_implicit_subscription to the put_op_intent function in F3548's dss.py to allow callers to create an OIR without any implicit subscription (which is allowed for OIRs in the ACCEPTED state)

This scenario covers the implicit cleanup fix on the DSS repo and covers some of the corner cases mentioned in interuss/dss#1088

@Shastick Shastick force-pushed the 415-implicit-sub-handling-scenario branch 4 times, most recently from 7f398f0 to 320a048 Compare July 19, 2024 17:14
@Shastick Shastick force-pushed the 415-implicit-sub-handling-scenario branch from 320a048 to bcc58d4 Compare August 23, 2024 13:44
@Shastick Shastick force-pushed the 415-implicit-sub-handling-scenario branch 2 times, most recently from 894d49a to 5049594 Compare September 10, 2024 09:00
@Shastick Shastick marked this pull request as ready for review September 10, 2024 09:16
@Shastick Shastick force-pushed the 415-implicit-sub-handling-scenario branch 3 times, most recently from 6cd5357 to 5b95b39 Compare September 10, 2024 10:44
@Shastick Shastick force-pushed the 415-implicit-sub-handling-scenario branch from 5b95b39 to 4bb3b4d Compare September 11, 2024 09:16
@@ -375,10 +375,17 @@ def constraints(
class FetchedSubscription(fetch.Query):
@property
def success(self) -> bool:
"""
Returns: True if the query was successful OR if it returned a 404.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think a FetchedSubscription should be considered successful if it returned a 404 (in that case, the subscription was not fetched successfully). I think we can make a was_not_found property (or similar) that we can check in conjunction with success to determine the full state.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That sounds like a good idea. I'll split this out to another PR, as I believe that not considering 404 a success will require some adaptations in other scenarios

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change has been split out to #785


## 🛑 Implicit subscription has correct temporal parameters check

If the implicit subscription time boundaries do not match the OIR's, either one, or both, of the following are possible:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If the implicit subscription time boundaries do not match the OIR's, either one, or both, of the following are possible:
If the implicit subscription time boundaries do not cover the OIR's, either one, or both, of the following are possible:

If the implicit subscription time boundaries do not match the OIR's, either one, or both, of the following are possible:

The DSS is in violation of **[astm.f3548.v21.DSS0005,1](../../../../../../requirements/astm/f3548/v21.md)**, as the implicit subscription does not cover the OIR's time boundaries;
Entities that should have been cleaned up earlier are still present in the DSS, and this scenario cannot proceed.
Copy link
Member

Choose a reason for hiding this comment

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

We should be able to differentiate between these two causes, and the requirement not to have cleaned-up entities in the DSS should be identified. Presumably, this is the ClearArea requirement, and presumably that check was already performed when clearing the area (so we don't need to refer to it again here since we know it's not the cause since the earlier clearing succeeded).

@Shastick Shastick force-pushed the 415-implicit-sub-handling-scenario branch from c847d78 to f09bb70 Compare September 20, 2024 16:49
@BenjaminPelletier BenjaminPelletier merged commit b23afd6 into interuss:main Sep 23, 2024
20 checks passed
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.

2 participants