Re: [PR] [incubator-kie-issues#1474] Fix NPE when loading multiple Signavio MID [incubator-kie-drools]
gitgabrio commented on PR #6080: URL: https://github.com/apache/incubator-kie-drools/pull/6080#issuecomment-2601830907 @yesamer Q125 ? 🤔 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] [incubator-kie-issues#1474] Fix NPE when loading multiple Signavio MID [incubator-kie-drools]
yesamer commented on PR #6080: URL: https://github.com/apache/incubator-kie-drools/pull/6080#issuecomment-2601797140 Hi @samuel-beniamin, we started the discussion about the release of 10.1.0 in the previous week (that will contain your fix) Despite a released date is still not defined, I would be surprised if 10.1.0 will land after Q125. Please stay tuned in the next weeks :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] [incubator-kie-issues#1474] Fix NPE when loading multiple Signavio MID [incubator-kie-drools]
samuel-beniamin commented on PR #6080: URL: https://github.com/apache/incubator-kie-drools/pull/6080#issuecomment-2601772114 Hello @baldimir, @yesamer and @gitgabrio First, thank you all for your answers. Unfortunately we have to wait in that case, as we are not the consumers of that library. Our customers are relying on it, and hence we can't just use the dependency as an m2 patch locally, but in fact wait for the next release. I am still wondering if the next release is planned for a date or even an estimated quarter? Any hint would be greatly appreciated. Thank you all again 🙇 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] [incubator-kie-issues#1474] Fix NPE when loading multiple Signavio MID [incubator-kie-drools]
baldimir commented on PR #6080: URL: https://github.com/apache/incubator-kie-drools/pull/6080#issuecomment-2582003819 Hi, correct, what @yesamer wrote. @samuel-beniamin If you cannot wait for the next release, you could backport your fix locally to the 10.0.x branch and you could build a patched jar meanwhile. It should be just a matter of building with Maven (mvn clean install). If you need to use it with multiple people, you would need to upload it to your Maven repository or each person copy it to their m2 local repositories. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] [incubator-kie-issues#1474] Fix NPE when loading multiple Signavio MID [incubator-kie-drools]
yesamer commented on PR #6080: URL: https://github.com/apache/incubator-kie-drools/pull/6080#issuecomment-2581990839 @samuel-beniamin @gitgabrio IINW, we branched 10 version before @samuel-beniamin PR, as you may notice here --> https://github.com/apache/incubator-kie-drools/commits/10.0.x/ It seems that we created the 10 branch on Jul 16, 2024, and @samuel-beniamin's PR was not backported in that branch, as was merged on Sep 13, 2024. I'm afraid you need to wait for the next release to have your changes released. I guess you will not be happy about that @samuel-beniamin, I apologize for that, but as @gitgabrio said 10 release was a nightmare -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] [incubator-kie-issues#1474] Fix NPE when loading multiple Signavio MID [incubator-kie-drools]
gitgabrio commented on PR #6080: URL: https://github.com/apache/incubator-kie-drools/pull/6080#issuecomment-2581968201 Sorry @samuel-beniamin the 10 release has been slightly cumbersome due to the big changes we had in the meantime, so probably I'm confusing date. @baldimir @yesamer, could you please help here ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] [incubator-kie-issues#1474] Fix NPE when loading multiple Signavio MID [incubator-kie-drools]
samuel-beniamin commented on PR #6080: URL: https://github.com/apache/incubator-kie-drools/pull/6080#issuecomment-2580755852 > Hi @samuel-beniamin this PR has been merged, and IINW it should already be in 10.0 release: am I missing something ? I tried to check both on 1. The released JAR file in maven for version 10.0.0 https://mvnrepository.com/artifact/org.kie/kie-dmn-signavio/10.0.0 and it didn't include the fix. 2. The branch 10.0.x it didn't include this fix or commit. Assuming I am not wrong, what would be next release to include this commit? and what can I do or assist with from my side? If I am wrong, could you please point me to the public release of the JAR file? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] [incubator-kie-issues#1474] Fix NPE when loading multiple Signavio MID [incubator-kie-drools]
gitgabrio commented on PR #6080: URL: https://github.com/apache/incubator-kie-drools/pull/6080#issuecomment-2580467388 Hi @samuel-beniamin this PR has been merged, and IINW it should already be in 10.0 release: am I missing something ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] [incubator-kie-issues#1474] Fix NPE when loading multiple Signavio MID [incubator-kie-drools]
samuel-beniamin commented on PR #6080: URL: https://github.com/apache/incubator-kie-drools/pull/6080#issuecomment-2580425666 Hey @gitgabrio, I am wondering how can I move this fix to be included in the next planned release? And what is the next planned release? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] [incubator-kie-issues#1474] Fix NPE when loading multiple Signavio MID [incubator-kie-drools]
gitgabrio commented on PR #6080: URL: https://github.com/apache/incubator-kie-drools/pull/6080#issuecomment-2348921947 Great @samuel-beniamin , many thanks!!! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] [incubator-kie-issues#1474] Fix NPE when loading multiple Signavio MID [incubator-kie-drools]
gitgabrio merged PR #6080: URL: https://github.com/apache/incubator-kie-drools/pull/6080 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] [incubator-kie-issues#1474] Fix NPE when loading multiple Signavio MID [incubator-kie-drools]
samuel-beniamin commented on PR #6080: URL: https://github.com/apache/incubator-kie-drools/pull/6080#issuecomment-2348856767 Here is a ticket to address a proper fix in the future https://github.com/apache/incubator-kie-issues/issues/1478 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] [incubator-kie-issues#1474] Fix NPE when loading multiple Signavio MID [incubator-kie-drools]
samuel-beniamin commented on PR #6080: URL: https://github.com/apache/incubator-kie-drools/pull/6080#issuecomment-2348246511 @gitgabrio Thank you for approving the the PR. > this seems more a workaround for a symptom of a problem somewhere else. Yes, I agree too, I think a proper fix should be to validate for which model is the call back being executed by the compiler itself. I will create another ticket for it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] [incubator-kie-issues#1474] Fix NPE when loading multiple Signavio MID [incubator-kie-drools]
gitgabrio commented on PR #6080: URL: https://github.com/apache/incubator-kie-drools/pull/6080#issuecomment-2346585222 @samuel-beniamin first of all, thanks for the PR ❤️ And then, thanks for explanation - my bad, I was confused by the log. I see your point, and this PR is fine for me. But TBH, we did not touch this module for some time, so we would need to dig in it a bit; and as you wrote > but maybe the proper fix should be in the DMNCompilerImpl. this seems more a workaround for a symptom of a problem somewhere else. So, if you may, I would kindly ask you to create another ticket (or duplicate the original one) so that we could focus on the root problem, eventually with your help : wdyt ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] [incubator-kie-issues#1474] Fix NPE when loading multiple Signavio MID [incubator-kie-drools]
samuel-beniamin commented on PR #6080: URL: https://github.com/apache/incubator-kie-drools/pull/6080#issuecomment-2346419025 @gitgabrio So skipping the execution of the callback in the specific case of Signavio's extension and in the case it is a MID executing callback for a different model, is what you are working on right now? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] [incubator-kie-issues#1474] Fix NPE when loading multiple Signavio MID [incubator-kie-drools]
samuel-beniamin commented on code in PR #6080:
URL:
https://github.com/apache/incubator-kie-drools/pull/6080#discussion_r1756947397
##
kie-dmn/kie-dmn-signavio/src/test/java/org/kie/dmn/signavio/SignavioTest.java:
##
@@ -291,31 +293,45 @@ void signavioIterateMultiinstanceWithComplexInputs() {
@Test
void signavioIterateMultiinstanceMultipleDecisions() {
DMNRuntime runtime = createRuntime("MID with multiple inside
decisions.dmn");
-
+
DMNContext context = runtime.newContext();
context.set("names", Arrays.asList("John", "Alice"));
-
+
DMNModel model0 = runtime.getModels().get(0);
LOG.info("EVALUATE ALL:");
DMNResult evaluateAll = runtime.evaluateAll(model0, context);
LOG.info("{}", evaluateAll);
-
+
assertThat(evaluateAll.getDecisionResultByName("overallage").getResult()).isEqualTo(new
BigDecimal("18"));
}
@Test
void signavioIterateMultiinstanceMultipleDecisionsOutside() {
DMNRuntime runtime = createRuntime("MID with outside requirement.dmn");
-
+
DMNContext context = runtime.newContext();
context.set("numbers", Arrays.asList(1,2,3));
context.set("operand", "PLUS");
-
+
DMNModel model0 = runtime.getModels().get(0);
LOG.info("EVALUATE ALL:");
DMNResult evaluateAll = runtime.evaluateAll(model0, context);
LOG.info("{}", evaluateAll);
-
+
assertThat(evaluateAll.getDecisionResultByName("sumUp").getResult()).isEqualTo(new
BigDecimal("6"));
}
+
+@Test
+void signavioMultiInstanceDecisionTableWithinMultipleFiles() {
+DMNRuntime dmnRuntime = createRuntime("MID with outside
requirement.dmn", "survey MID SUM.dmn", "Signavio_Concat.dmn");
+
+List decisionNames = dmnRuntime
+.getModels()
+.stream()
+.flatMap(dmnModel -> dmnModel.getDecisions().stream())
+.map(DMNNode::getName)
+.collect(Collectors.toList());
+
+assertThat(decisionNames).containsOnly("sumUp", "iterating",
"determineModifier", "concatNames");
Review Comment:
Looks good, applied ✅
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org
For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] [incubator-kie-issues#1474] Fix NPE when loading multiple Signavio MID [incubator-kie-drools]
samuel-beniamin commented on PR #6080:
URL:
https://github.com/apache/incubator-kie-drools/pull/6080#issuecomment-2346365889
> Hi @samuel-beniamin Reading the original ticket, it seems there is some
sort of issue in the implementation. But IINW, this PR does not change the
implementation at all (if just add a log), and rather modifies a test: could
you clarify that ?
Hello @gitgabrio, It is in the implementation, I can still see my change
returning or exiting early incase the model is not the same.
https://github.com/user-attachments/assets/7d01f02a-1c93-487c-977f-fc698d2c6e21";>
So basically, the compiler calls this callback regardless of the model it is
trying to evaluate, but in this case the extra check is basically the fix, it
seems trivial but it is blocking if we need to load multiple models.
```java
if (cModel != model) {
return;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org
For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] [incubator-kie-issues#1474] Fix NPE when loading multiple Signavio MID [incubator-kie-drools]
gitgabrio commented on PR #6080: URL: https://github.com/apache/incubator-kie-drools/pull/6080#issuecomment-2346183602 Hi @samuel-beniamin Reading the original ticket, it seems there is some sort of issue in the implementation. But IINW, this PR does not change the implementation at all (if just add a log), and rather modifies a test: could you clarify that ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] [incubator-kie-issues#1474] Fix NPE when loading multiple Signavio MID [incubator-kie-drools]
pibizza commented on code in PR #6080:
URL:
https://github.com/apache/incubator-kie-drools/pull/6080#discussion_r1756779320
##
kie-dmn/kie-dmn-signavio/src/test/java/org/kie/dmn/signavio/SignavioTest.java:
##
@@ -291,31 +293,45 @@ void signavioIterateMultiinstanceWithComplexInputs() {
@Test
void signavioIterateMultiinstanceMultipleDecisions() {
DMNRuntime runtime = createRuntime("MID with multiple inside
decisions.dmn");
-
+
DMNContext context = runtime.newContext();
context.set("names", Arrays.asList("John", "Alice"));
-
+
DMNModel model0 = runtime.getModels().get(0);
LOG.info("EVALUATE ALL:");
DMNResult evaluateAll = runtime.evaluateAll(model0, context);
LOG.info("{}", evaluateAll);
-
+
assertThat(evaluateAll.getDecisionResultByName("overallage").getResult()).isEqualTo(new
BigDecimal("18"));
}
@Test
void signavioIterateMultiinstanceMultipleDecisionsOutside() {
DMNRuntime runtime = createRuntime("MID with outside requirement.dmn");
-
+
DMNContext context = runtime.newContext();
context.set("numbers", Arrays.asList(1,2,3));
context.set("operand", "PLUS");
-
+
DMNModel model0 = runtime.getModels().get(0);
LOG.info("EVALUATE ALL:");
DMNResult evaluateAll = runtime.evaluateAll(model0, context);
LOG.info("{}", evaluateAll);
-
+
assertThat(evaluateAll.getDecisionResultByName("sumUp").getResult()).isEqualTo(new
BigDecimal("6"));
}
+
+@Test
+void signavioMultiInstanceDecisionTableWithinMultipleFiles() {
+DMNRuntime dmnRuntime = createRuntime("MID with outside
requirement.dmn", "survey MID SUM.dmn", "Signavio_Concat.dmn");
+
+List decisionNames = dmnRuntime
+.getModels()
+.stream()
+.flatMap(dmnModel -> dmnModel.getDecisions().stream())
+.map(DMNNode::getName)
+.collect(Collectors.toList());
+
+assertThat(decisionNames).containsOnly("sumUp", "iterating",
"determineModifier", "concatNames");
Review Comment:
Maybe you can simply this and the previous statement using directly assertj
assertions. Assertj has a powerful fluent interface to manipulate streams and
perform stream operation. You can check extracting and flatExtracting, for
example.
```suggestion
assertThat(dmnRuntime.getModels()).flatExtracting(dmnModel ->
dmnModel.getDecisions()).extracting(DMNNode::getName).containsOnly("sumUp",
"iterating", "determineModifier", "concatNames");
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org
For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] [incubator-kie-issues#1474] Fix NPE when loading multiple Signavio MID [incubator-kie-drools]
pibizza commented on PR #6080: URL: https://github.com/apache/incubator-kie-drools/pull/6080#issuecomment-2346167664 Added @gitgabrio, he is working on this stuff at the moment. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
