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

[DROOLS-7635] ansible-rulebook : Raise an error when a condition compares incompatible types #114

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tkobayas
Copy link
Collaborator

This draft PR requires
apache/incubator-kie-drools#6104
to be built beforhand.

Cross repo PR check doesn't work for now.

…ares incompatible types

- WIP: This doesn't support alpha index. Some tests fail
- support node sharing (but not merging error messages)
- WIP: need more assertions
@tkobayas
Copy link
Collaborator Author

tkobayas commented Sep 27, 2024

@mariofusco
With apache/incubator-kie-drools#6104 , now alpha index and node sharing is supported.

Limitation: in case of node sharing, the error message has only one ruleSetName/ruleName/conditionExpression. It means that when a user fixes a condition based on the error message, they may hit the similar issue in another condition next time. It might be invasive to "combine" error messages upon node sharing in drools code base. We may ask Madhu if it's important.

Feel free to share your thought, thanks!

@tkobayas
Copy link
Collaborator Author

The point I mentioned at #112 (comment) still remains

  • Alpha index skips unmatched AlphaNodes and jumps to the right AlphaNode. That means, when an event doesn't match any AlphaNode because of type mismatch , the custom predicate would not be executed. This is actually the case that users want to read the error message ... "No rule fires, I don't know if constraints are wrong". I think we would need to add the check logic in CompositeObjectSinkAdapter.propagateAssertObject.

I'll need to consider further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant