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

Add External Events #1513

Open
wants to merge 88 commits into
base: develop
Choose a base branch
from
Open

Add External Events #1513

wants to merge 88 commits into from

Conversation

pranav-super
Copy link
Contributor

@pranav-super pranav-super commented Jul 23, 2024

  • Tickets addressed: MPS-118
  • Review: By commit
  • Merge strategy: Merge (no squash)

Description

A new construct in AERIE is that of External Events. In this PR, we will introduce External Events, their containing entities (External Sources), their grouping mechanisms (Derivation Groups), as well as how they manifest in the UI and in the backend. Detail on what External Events are is provided in the aerie-ui PR, but as a quick summary:

An external event is a construct that represents the occurrence of something outside of what is modeled in the mission model. They could be events that are close to the spacecraft and directly affect behavior on it, like a DSN visibility event that might affect when communications happen, or they could be much more remote, like a constellation event. Functionally, these are at the interstice between external datasets and activities, in that they manifest on the timeline as activities (discrete blocks of time), but they themselves do not affect the mission model or invoke any actions much like external datasets.

One component of that discussion that is deferred to this description, as it is here that it is primarily handled, is that of derivation. We discuss that in detail here.

Derivation

Derivation was an operation conceived for the reconciliation of overlapping External Sources. The problem at hand was that it is entirely possible for sources to be generated at later points in time, that cover slightly different windows on plan time than their predecessors.

image

If this is the case, some reconciliation scheme is needed to determine which External Events from these External Sources are valid and should be visible on the timeline.

Reconiciliation Mechanism

To begin this discussion, we should first talk about when External Sources apply.

External Sources, as discussed earlier, have within them a valid_at field, which was described as a more particular form of a version number. With this, we know we can impose an ordering of External Sources. Given that ordering, figuring out when each External Source actually applies is as simple as determining for a given time (or slot of time), what the most recently valid External Source is. For example, the following set of 4 External Sources would be applicable in the following order. Note that one of the Exteranl Sources, B, is valid twice, as it is the most recently valid External Source for a given slot of time:

image

Given this, we can now discuss how to compare the External Events within each External Source, based on how they fall across these windows/against future External Sources. There are 4 rules (though the first two are effectively cases of each other):

  1. An External Event superceded by nothing will be present in the final, derived result. The logic behind this is that this event has no future data that might challenge its validity, so it reasonably would trickle down as an event in a final result.
  2. An External Event partially superceded by a later External Source, but whose start time occurs before the start of said External Source(s), will be present in the final, derived result. The logic behind this is similar. This event, at its start, has no future data that might challenge its validity. Because its start is still considered valid, we accept the event in its entirety to the final, derived result, as no future External Source that could overlap with this External Event could place an event that applies to the time that this External Event starts at. If it did, we have a case of rule 3.
  3. An External Event whose start is superseded by another External Source, even if its end occurs after the end of said External Source, will be replaced by the contents of that External Source (whether they are blank spaces, or other events). The logic here is simply that we now do have more recent data, so we can safely do a complete replacement. Should an External Event be replaced despite ending after the original source, we can still justify this as the updated External Source, if it had need for such an External Event, would have included it.
  4. An External Event who shares an ID with an External Event in a later External Source will always be replaced. By introducing naming, we can allow replacement.

A diagram illustrating each of these cases follows. We also have External Sources formatted to align with this diagram here.

image

Derivation Groups

The final question remaining is the 'who' - who is this derivation operation run against? One possible candidate for this could be the source type. External Events that share a source type include events of the same types and will likely overlap in key namespace as well. This is a nearly ideal solution except in the case of contingencies.

Imagine we have 4 sources of the same source type. Two of these relate to an ideal contingency, and two of these relate to a degenerate contingency:

  • Ideal
    • DSN_Contact_Ideal_2026001007.json
    • DSN_Contact_Ideal_2026003010.json
  • Degenerate
    • DSN_Contact_Degen_2026001007.json
    • DSN_Contact_Degen_202603010.json

If we were to derive solely based on source type, we would effectively be stacking all of these sources against each other to produce one result:

image

Given that these are different cases, despite sharing a source type, it would be useful to have different groups under which derivation is possible. This way, we don't conflate sources just for sharing a source type:

image

As such, we now define the notion of a Derivation Group. A Derivation Group defines a subgroup within an External Source type of External Sources to derive against each other. Derivation Groups are what we associate with plans (as opposed to individual, underived External Sources, or entire External Source types).

Verification

Automated tests were primarily added in aerie-ui. By adding tests here and ensuring that all effects as well as all new UI features worked as intended, we believe that we have sufficiently tested all new functionality as the majority of it deals with either the front-end itself, or new tables and views. By testing that our requests to Hasura from the front-end work via said tests, we believe we have thoroughly tested AERIE's new functionality.

Documentation

No documentation was invalidated. That being said, new documentation is in the works and will eventually see a PR against aerie-docs.

Future work

The next major step is to add integration with the scheduler and the constraints engine to allow scheduling and constraining of activities and resources relative to uploaded External Events. We also seek at some point to define a comprehensive JSON Schema for External Sources and External Events, which was deferred until after this batch so that we had a very concrete and informed view of the problem prior to defining any constraining design. Finally, we might want to introduce an ownership and role scheme around External Sources, though the utility of this is yet to be confirmed and agreed upon.

Discussion Items and TODOS:

Discussion Items

  • Squash tables for source type, event type, and derivation group?
  • Discuss valid_at - should it be called valid_after, created_at, published_at, etc.
  • Parsing can/should be merged to Aerie Gateway to use ‘common’ parsing code?
  • Ideal branching behavior?
  • Cherrypicking of events/individual replacement of events outside of window?

TODOS:

  • Remove cleanup - we can let source types and event types linger. Simply verify that empty derivation groups/types can still be readded to and that we don't create a new one. Should have a way to delete these if they are empty?
  • make sure permissions in db prevent inadvertent mutability
  • make the trigger to check derivation group and source type of uploaded source a foreign key
  • get rid of ownership stuff in SQL - its actually user specific the way we implemented it
  • Just parse and then discard the files!!!
  • (MAYBE) convert all keys to composite instead of the serial ids
  • add to db-tests
    • do this after we deliberate any new rules (if we do - consider simplicity)
    • add tests for derived event view and derivation group comp view
  • Squash all migrations - final step before merge
  • make view for external_source_event_type just a graphql query instead as this is faster

@pranav-super pranav-super requested a review from a team as a code owner July 23, 2024 18:32
Not completely confident on placing it in external_event.sql, open to comment there.
add duration to derived_events (helpful quantity to include without need for cross-referencing), change check on end time for source to be > start time, not >=
Was not pointing to external_event pkey!
Branching, for now, is made up of 2 behaviors:
 - when a new branch is created, all associated derivation groups in the parent are copied to the child
 - on merge, the parent gets a superset (OR operation) of the derivation groups the parent and child have. The child is unaffected.
properties jsonb,

constraint external_event_pkey
primary key (key, source_key, derivation_group_name, event_type_name),
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a fan of large composite primary keys, especially when one or all of the components are strings. Please make a column id integer generated by default as identity and set that as the primary key. I don't think it'll be a very impactful change.

Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't had a chance to take a look and form an opinion, but I'd like to leave as a reminder that this pk would need to be replaced with a unique constraint if this change is made.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe Pranav et al started with synthetic keys for everything, and then based on an initial review (with @Mythicaeda ?) refactored everything to use natural keys instead.

Sounds like maybe there's a middle ground where sometimes we prefer one or the other? @JoelCourtney and @Mythicaeda , if you have some time, talk this over amongst yourselves and see if you can come to a consensus on the guidance to give Pranav

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
publish Tells GH to publish docker images for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants