Track constraint checking #1013
Merged
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
camargo
added
feature
new file: merlin-server/sql/merlin/tables/constraint_run.sql new file: merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/ConstraintRepository.java new file: merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/ConstraintRunRecord.java new file: merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/CreateConstraintRunAction.java new file: merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/GetConstraintRunsAction.java new file: merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/InsertConstraintRunsAction.java new file: merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/PostgresConstraintRepository.java new file: merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/ConstraintAction.java new file: merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/ConstraintService.java new file: merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/LocalConstraintService.java new file: merlin-server/src/test/java/gov/nasa/jpl/aerie/merlin/server/mocks/StubConstraintService.java
cohansen
force-pushed
the
feature/track-constraint-checking
branch
from
8969a15 to
8fe0b14
Compare
camargo
requested review from
camargo and
skovati
and removed request for
mattdailis and
jmdelfa
camargo
approved these changes
Jul 11, 2023
There was a problem hiding this comment.
Works great! Tested with a couple constraints in the demo mission model. Created NASA-AMMOS/aerie-ui#770 to update the UI to use the new table. Good to go.
Mythicaeda
reviewed
Jul 11, 2023
deployment/hasura/metadata/databases/AerieMerlin/tables/public_constraint_run.yaml
Show resolved
Hide resolved
Mythicaeda
reviewed
Jul 11, 2023
merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/http/MerlinBindings.java
Show resolved
Hide resolved
Mythicaeda
reviewed
Jul 11, 2023
merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/ConstraintAction.java
Outdated
Show resolved
Hide resolved
Thanks Chris! |
…taset to constraint run
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.
Closes #997
Closes #514
Closes #637
Description
This PR adds constraint run tracking. The way it's currently working is each
constraint_runrecord is tied to a constraint that was checked. Regardless on if that constraint had violations or not we save a record to theconstraint_runtable that includes basic constraint information and the violations that were encountered. This allows us to reuse those violations in subsequent constraint violation checks.There is also staleness checking for the runs through a field called
definition_outdated. This field tracks if the constraint definition has changed since the violations were checked last.There is also staleness checking for the runs, each record gets the statusresolvedwhen it has been saved to the db. If the user makes a change to a constraint with anyresolvedruns, those affected runs get a status change toconstraint-outdated. This works the same way for simulations, if a violation check happened and then the user runs a new simulation all constraint runs for that previous dataset are marked assimulation-outdated.This is my first big PR for
aerie-merlinso please hit me with some feedback!Verification
Manual tests, I need to work on adding some real tests but I wanted to get the PR up for review first. Expect another commit with tests.
To verify everything is working properly you can do the following:
Add a constraint to the plan (I used
return Real.Resource('/peel').greaterThan(x);for a lot of my testing).constraint_runtable now should be populated with a single entry and it shouldn't be marked as outdated.definition_outdatedfield should now betrue.1. Create a model and a plan.2. Add a constraint to the plan (I used
return Real.Resource('/peel').greaterThan(x);for a lot of my testing).3. Add an activity to the plan and simulate.
4. Do a constraint violation check, the
constraint_runtable now should be populated with a single entry with a statusresolved.7. Go modify your constraint, that entry in the db should be now marked as
constraint-outdated.8. Do another constraint violation check, a new entry should exist in the table with a status
resolved.9. Add another activity and re-simulate.
10. The newest entry in the
constraint_runtable should now be markedsimulation-outdated.You can also run the constraint violation check in succession to test the caching behavior.
Documentation
I'll update aerie docs to add information about constraint run tracking after this PR is created. Link will be added here.
Future work
Going to work on #514 as a separate ticket.