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

[scd] dangling implicit subscription when OIR with implicit subscription is mutated #1056

Closed
Shastick opened this issue Jul 18, 2024 · 2 comments
Assignees

Comments

@Shastick
Copy link
Contributor

Shastick commented Jul 18, 2024

Describe the bug

When updating an Operational Intent Reference (OIR) for which an implicit subscription was created, if the subscription id is not specified and the parameters for creating an implicit subscription are specified, the DSS will create a new implicit subscription without deleting the previous one.

This is the region of the code where it happens.

To Reproduce

  1. Create an OIR and request the creation of an implicit subscription
  2. Mutate the OIR without specifying a subscription ID and specify some parameters for an implicit subscription
  3. Observe that the returned OIR has a new implicit subscription ID
  4. Query the previous implicit subscription: it still exists, even if the OIR is now attached to the newly created implicit subscription

Expected behavior

After the mutation of an OIR with an implicit subscription, under all circumstances, either the existing implicit subscription has been reused and properly updated, or a new implicit subscription has been created, and the previous one removed if it is no longer attached to any OIR.

Additional questions/discussion

Options to solve this:

  • A) On mutation, when no subscription ID is specified and the parameters for an implicit subscription are specified, it is acceptable to reuse the existing implicit subscription and mutate it if necessary.

    • Important: with this approach, the DSS should make sure that the callback URL of the subscription is also updated if the one passed in the implicit parameters of the mutation differs from the original one. Or, if this is not allowable, it should return an error.
  • B) On mutation, when no subscription ID is specified and the parameters for an implicit subscription are specified, delete the previous implicit subscription, if it exists and has no more OIRs attached, and create a new one.

I'm in favor of option B here, as I'm not sure that allowing to indirectly mutate the callback URL of an implicit subscription is a desirable feature:

  • callers that wish to mutate the callback URL of a subscription may mutate it directly.
  • callers that desire that an implicit subscription be re-used can specify the relevant subscription ID in the mutation request

Side question: On mutation, when no subscription and no implicit subscription parameters are specified, should we:

  • delete the existing implicit subscription if we are allowed to (state is ACCEPTED), return an error if we are not allowed
  • mutate the existing implicit subscription, if one exists and if it is required

Another way to look at this question is to ask how do we remove an implicit subscription from an OIR in the ACCEPTED state without deleting the OIR and re-creating it?

  • Is this something we would wish to support?
  • I would need to confirm this, but I believe that deleting the implicit subscription would not work (I expect that the logic checking whether a subscription is still covering an OIR to ignore the OIR's state)
@Shastick
Copy link
Contributor Author

@BenjaminPelletier FYI I'll start working on this (but can't assign myself)

@Shastick Shastick self-assigned this Aug 30, 2024
Shastick added a commit to Orbitalize/dss that referenced this issue Sep 9, 2024
This is a rework of interuss#1057 after most changes for interuss#1088 have landed.

The DSS built from this PR successfully passed the new qualifier's implicit subscription handling scenario, although that scenario still needs to be extended to cover all corner cases.

The PR is open for review, as it addresses interuss#1056
mickmis pushed a commit that referenced this issue Sep 9, 2024
This is a rework of #1057 after most changes for #1088 have landed.

The DSS built from this PR successfully passed the new qualifier's implicit subscription handling scenario, although that scenario still needs to be extended to cover all corner cases.

The PR is open for review, as it addresses #1056
@Shastick
Copy link
Contributor Author

This has been addressed by #1109

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 a pull request may close this issue.

1 participant