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

Return all Constraint during Compilation #1242

Merged
merged 6 commits into from
Nov 21, 2023

Conversation

goetzrrGit
Copy link
Contributor

@goetzrrGit goetzrrGit commented Nov 9, 2023

Description

Revise the ConstraintAction to avoid immediate termination and exception throwing in the event of a compilation error with a constraint. Instead, store these errors in a Failable object, enabling us to retrieve the information later for all constraints. Exceptions are retained for situations where it's illogical to proceed with checking a list of constraints without a plan, mission model, or simulation dataset.

I've also addressed an issue where a constraint previously compiled and ran successfully, but encountered compilation issues after a user modification. Now, when the user rectifies the constraint and it successfully compiles and runs, the SQL throws an exception saying the constraint already exists is primary_id. Now we do an update on collision.

Verification

A lot of manual testing and e2e test.

Future work

Add badges to constraints that have compilation errors.

@goetzrrGit goetzrrGit requested a review from a team as a code owner November 9, 2023 19:24
@goetzrrGit goetzrrGit changed the title Return all Constraint Compilation Return all Constraint during Compilation Nov 9, 2023
@goetzrrGit goetzrrGit added constraints Anything related to the constraints domain bug Something isn't working database Anything related to the database clipper Requests from the Europa Clipper project labels Nov 9, 2023
@goetzrrGit goetzrrGit force-pushed the 1194--Full_constraint_compilation branch from 28f93af to fa1f25e Compare November 10, 2023 20:16
@goetzrrGit goetzrrGit force-pushed the 1194--Full_constraint_compilation branch from fa1f25e to 55f2dbb Compare November 10, 2023 20:25
@goetzrrGit goetzrrGit force-pushed the 1194--Full_constraint_compilation branch from 55f2dbb to c1dece0 Compare November 10, 2023 20:35
@goetzrrGit goetzrrGit force-pushed the 1194--Full_constraint_compilation branch from c1dece0 to 957abf3 Compare November 14, 2023 22:04
@goetzrrGit goetzrrGit force-pushed the 1194--Full_constraint_compilation branch from 957abf3 to 35387eb Compare November 15, 2023 18:25
@goetzrrGit
Copy link
Contributor Author

@Mythicaeda I believe I fixed everything you suggested in the ConstraintTest. I have updated all the tests with optional checking and did some parameter renaming to make it more clear.

Copy link
Contributor

@Mythicaeda Mythicaeda left a comment

Choose a reason for hiding this comment

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

LGTM!

The Failable class facilitates the storage of an object along with a status indicating success or failure.
With the integration of the Failable class, I've eliminated the short-circuit behavior, enabling all constraints to be executed even in the presence of an error. However, exceptions are still raised when either no plan exists or the simulation dataset is absent, as attempting to run a list of constraints without sufficient data would serve no purpose.
This fix addresses a bug where a constraint that initially works, is subsequently modified and fails during execution. Previously, attempting to insert the updated constraint would result in an error due to the existing constraint in the table. With this modification, we now override the entry if it is successful again, ensuring smooth execution.
With the implementation of the Failable class, there is no longer a need to throw the ConstraintCompilationException. This exception was previously used to handle compilation errors, but now we capture and handle the data using the Failable class, streamlining the process.
Process the captured Failable and construct a JSON object.
* We are always returning an object now

* Update error messages.
@goetzrrGit goetzrrGit force-pushed the 1194--Full_constraint_compilation branch from 54fc73f to bcc40c4 Compare November 21, 2023 22:09
@goetzrrGit goetzrrGit merged commit 2b90a43 into develop Nov 21, 2023
10 checks passed
@goetzrrGit goetzrrGit deleted the 1194--Full_constraint_compilation branch November 21, 2023 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working clipper Requests from the Europa Clipper project constraints Anything related to the constraints domain database Anything related to the database
Projects
None yet
2 participants