-
Notifications
You must be signed in to change notification settings - Fork 39
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
Validator Tracing and TimeTriggeredValidator #524
Conversation
Commenting just to say I would like to see this implemented - I have some experiments this could really help with. |
In pr #509 also happens a "transformation" of a temporal structure into a non-temporal one. After the merge of both there will most certainly be space for refactoring (that maybe can be considered also when designing the missing code). |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #524 +/- ##
==========================================
- Coverage 84.96% 84.92% -0.04%
==========================================
Files 200 200
Lines 26433 26611 +178
==========================================
+ Hits 22458 22600 +142
- Misses 3975 4011 +36 ☔ View full report in Codecov by Sentry. |
9fc25cc
to
935ccff
Compare
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.
LGTM! 😃
I left 4 comments. 2 are minor code-level improvements, 1 I think it's a bug, the 4th one is just a note on the SequentialValidator
supported_kind
…red plan validator
a021a86
to
3ab3c31
Compare
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.
LGTM! I just added a small comment about an awkwardness in error handling.
In this PR we extend the internal sequential validation engine to return the sequence of states encountered by executing a sequential plan and we add support for validation (and tracing) of TimeTriggeredPlans