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

CAM-13207 DMN execution context #1357

Closed

Conversation

jangalinski
Copy link
Contributor

In order to allow hooking into the den decision evaluation, the engine should not create the context via new. Instead, it should use a factory which can be configured via the engineConfiguration.

See https://jira.camunda.com/browse/CAM-13207

- avoid creation via new ... intead allows configuration of factory. Default implementation remains untouched

related to CAM-13207
@jangalinski
Copy link
Contributor Author

I didn't provide tests so far ... but the default behavior did not change, so the existing tests will prove everything still works. Please get in touch to discuss which tests I should provide (and how I can set up my local build ... currently, I cannot run tests).

@jangalinski
Copy link
Contributor Author

Note: I used separate smaller commits instead of squashing them together, because while working on this, I fixed javadoc typos and added override annotations ... those commits should end up in master independently, imho.

@mboskamp
Copy link
Member

Hey @jangalinski, as mentioned in the CAM ticket. We will get back to you as soon as possible. Currently, the 7.15.0 release is taking up all the resources.

Cheers,
Miklas

@tmetzke
Copy link
Member

tmetzke commented Jun 14, 2021

Hey @jangalinski,

as mentioned in CAM-13207, we will close without a merge here. While the changes are valid and reasonable itself, you should be able to achieve the desired behavior already with the DmnDecisionEvaluationListener.

Let us know if we missed anything so we should reconsider.

Best,
Tobias

@tmetzke tmetzke closed this Jun 14, 2021
@jangalinski
Copy link
Contributor Author

No, I can't use DmnDecisionEvaluationListeners. That was the whole point of this PR. listeners are only implemented for decisionTables. In case of expressions and literal expressions, I do not have listeners. That's why I introduced this abstraction layer, so I can overwrite (aka fix) this missing behavior.

@jangalinski
Copy link
Contributor Author

Concrete use case: in a DRD that uses expressions and tables, I want to get the intermediate results of each sub-execution. Possible for tables, impossible for expressions.
If I do not get it right, could you show me a working example?

@tmetzke
Copy link
Member

tmetzke commented Jun 16, 2021

Hey Jan,

thanks for that feedback.

I just did a quick debug with a decision that consists of a literal expression and I can see that the registered DmnDecisionEvaluationListeners are informed about the decision result containing the literal expression information in the DefaultDmnDecisionContext. The DmnDecisionEvaluationListeners are invoked after the evaluation of a decision, no matter what is inside that decision (literal expression, decision table).

This might however not exactly cover your use case. Therefore, would you mind detailing what you would want to change in DefaultDmnDecisionContext so it provides the behavior you expect? It might also help us if you provide a specific example DMN that does not work as you expect. Please describe along that example what already works as expected and what doesn't. This will help us to better understand the specific use case.

Thanks and best,
Tobias

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.

None yet

3 participants