-
Notifications
You must be signed in to change notification settings - Fork 19
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 Hasura Events Views #1418
Merged
Merged
Add Hasura Events Views #1418
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Make Model Upload await logs rather than delay 1.5s - Fix inconsistency in SchedulingTests.beforeEach
Mythicaeda
added
database
Anything related to the database
hasura
Changes that affect the Hasura metadata
labels
Apr 24, 2024
Mythicaeda
force-pushed
the
feat/hasura-events-view
branch
from
April 24, 2024 17:46
7c7cef8
to
62890bc
Compare
Mythicaeda
force-pushed
the
feat/hasura-events-view
branch
from
April 24, 2024 17:50
62890bc
to
56a50b9
Compare
Mythicaeda
force-pushed
the
feat/hasura-events-view
branch
from
April 24, 2024 17:55
56a50b9
to
3954e7f
Compare
Mythicaeda
force-pushed
the
feat/hasura-events-view
branch
from
April 24, 2024 18:01
3954e7f
to
044abd6
Compare
Fixed the PGCMP Workflow. Checking out `develop` requires re-cloning the `PGCMP` repo Fixed a bug in the migration script that was causing hasura to reapply migration 1 when bulk migrating down from version 1. Fixed a typo in the migration script preventing the "mark correct version" command from applying
Mythicaeda
force-pushed
the
feat/hasura-events-view
branch
from
April 24, 2024 18:19
044abd6
to
926f50d
Compare
Mythicaeda
requested review from
adrienmaillard
and removed request for
cohansen
April 25, 2024 00:07
skovati
approved these changes
Apr 26, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Excited to have our first migration in the new DB!
Forgot to ask in my review, where is the migration script "typo" that you fixed? Just the "aerie" vs "Aerie"? |
Yeah, apparently Hasura's case sensitive 🥲 |
13 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Creates a SQL function to get the event logs for a specified Hasura event, and three views that select from this function, one for each of our Hasura events.
While the views could not be necessary had I actually created and tracked a custom table for the function return type in order to satisfy the Hasura Custom Function API (as is done with our other Hasura functions), by using views we can establish relationships between the results and the
mission_model
table, allowing for queries like so:Note that using a function for the view definitions is required, as the Hasura tables the views reference do not exist during database initialization (Hasura creates and manages them). View definitions must be executable at the time of creation, but functions do not have this restriction.
This PR also fixes some minor bugs in our Migrations pipeline. Specifically:
develop
requires re-cloning thePGCMP
repo. The workflow was doing this pre-2.8.0, but I accidentally removed it when updating it for 2.8.0.hasura
command is case-sensitive. This bug was introduced in 2.8.0.Verification
Manual verification was done to figure out the schema for errors and to check the Hasura metadata.
An E2E test was added to the
MissionModelTests
suite for testing the ability to get these logs. Additionally, the model upload process now awaits these logs, replacing theThread.sleep
inHasuraFunctions.uploadMissionModel
While checking if any tests were delaying for these events to complete, I noticed that
SchedulingTests.BeforeEach
was inconsistent in how it uploaded the model compared to other tests classes (it caught and rethrew the potentialInterruptedException
as aRuntimeException
). This has been resolved.Documentation
Docs ticket here: NASA-AMMOS/aerie-docs#147
Future work
A front-end ticket is needed to track displaying success/failures for model uploads.