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 #1396

Merged
merged 631 commits into from
Oct 22, 2024
Merged

Add External Events #1396

merged 631 commits into from
Oct 22, 2024

Conversation

pranav-super
Copy link
Contributor

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

___REQUIRES_AERIE_PR___="1513"

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. Extensive documentation on this will be added to aerie-docs, but a general introduction can be provided in this PR description.

External Events

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.

image

The need for these became apparent after an exploration of External Events implemented as activities and as resources. To mimic the same construct as a resource, we would need to parse the external data, and then place it on the timeline as a discrete resource, which given the fact that there are gaps between events and there can also be simultaneous events, proved extremely unwieldy and awkward. While activities were a more natural representation, we still required an intermediary of a resource to schedule these activities based off of an ingested file, as it isn't possible to schedule activities exclusively following information in a file.

image

As such, a new construct became envisioned, and that is what is introduced in this PR.

How?

External Events are bundled in files called External Sources. External Sources additionally contain some metadata about the collection of External Events (i.e. what period of time this External Source applies over). These can be uploaded to AERIE and managed in a newly added External Source Manager:

image

It is in this view that External Sources can be inspected, uploaded, and deleted. Both the metadata as well as the component External Events (via a Timeline or Table view) can be inspected:

External Events as a Table External Events as a Timeline
image image

Furthermore, grouped within another new construct (and operation) known as Derivation Groups, these External Sources and their External Events can be viewed on a timeline:

image

For the sake of brevity, we will not discuss these details further. Furthermore, details on the derivation operation can be found in the AERIE PR, as it is there that the majority of derivation-related logic is handled.

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:

  • Bugs:
    • UpdateCard in plan persists very unnecessary extra data somehow.
    • Tooltip becomes permanently sticky for some reason on the timeline (seen on ExternalSourceManager.svelte)
    • Verify that during file parsing/upload, it is just one transaction? It seems fine to create an external event type and then fail on creation of a source, so the ONLY thing we should worry about here is verifying that orphaned events (events without a source) are impossible
    • Sometimes nothing appears in the external sources panel - rare timing issue.
  • Features/Other:
    • 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 valid_at adhere to 8061. actually all dates if possible
    • Tests for 0 duration events
    • Just parse and then discard the files!!!
    • (MAYBE) change derivation group in the UI itself? (not yet?)
    • (MAYBE) Add handling in ExternalSourceManager if can't reach hasura, don't show toasts saying it worked and switch panels, instead fail out, give a message, cancel the outgoing request?
    • (MAYBE) add handling about views refreshing for user change in plan
    • (MAYBE) make sure filters select correctly in ExternalSourceManger.svelte on user change, add tests
    • Change spacing in timeline for ee's if not grouping, to not justify to bottom but just to add height+5 space between rows
    • fix name of query GetExternalEvents in gql.ts, update docs
    • 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:10
Copy link
Collaborator

@duranb duranb left a comment

Choose a reason for hiding this comment

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

Do you mind reverting all the changes made to the port and github workflows, please?

@pranav-super
Copy link
Contributor Author

Do you mind reverting all the changes made to the port and github workflows, please?

To be specific, do you mean all changes in .github/workflows? Or are there additional places too?
Also, I believe that in test.yml we needed the parts about caching; is that okay to leave in?

@duranb
Copy link
Collaborator

duranb commented Jul 24, 2024

Do you mind reverting all the changes made to the port and github workflows, please?

To be specific, do you mean all changes in .github/workflows? Or are there additional places too? Also, I believe that in test.yml we needed the parts about caching; is that okay to leave in?

I'd say just the added caching test step in the test.yml file is fine to keep, but all the other changes in that folder in general do not seem necessary to support the feature. I'm open to discussing more about it if you feel otherwise. The only changes outside that folder that should be reverted are the port changes from 3000 -> 3005.

@pranav-super
Copy link
Contributor Author

I'd say just the added caching test step in the test.yml file is fine to keep, but all the other changes in that folder in general do not seem necessary to support the feature. I'm open to discussing more about it if you feel otherwise. The only changes outside that folder that should be reverted are the port changes from 3000 -> 3005.

Taken care of!

README.md Outdated Show resolved Hide resolved
.github/workflows/lint.yml Outdated Show resolved Hide resolved
docs/DEVELOPER.md Outdated Show resolved Hide resolved
playwright.config.ts Outdated Show resolved Hide resolved
playwright.config.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
JosephVolosin pushed a commit that referenced this pull request Oct 21, 2024
The bulk grid action table was used to support the quick fix suggested here: #1396 (comment), and ultimately to support that table, multi-delete was implemented.
JosephVolosin pushed a commit that referenced this pull request Oct 21, 2024
@JosephVolosin JosephVolosin force-pushed the feature/external-event branch from 7cee587 to 4f383e0 Compare October 21, 2024 13:41
src/stores/external-event.ts Outdated Show resolved Hide resolved
src/stores/external-event.ts Outdated Show resolved Hide resolved
@duranb duranb added feature New feature or request DON'T MERGE Do Not Merge This Branch labels Oct 21, 2024
Copy link
Collaborator

@duranb duranb left a comment

Choose a reason for hiding this comment

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

Everything looks good to me! Thank you!

@AaronPlave AaronPlave removed the DON'T MERGE Do Not Merge This Branch label Oct 22, 2024
@JosephVolosin JosephVolosin merged commit 66b24bf into develop Oct 22, 2024
5 checks passed
@JosephVolosin JosephVolosin deleted the feature/external-event branch October 22, 2024 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants