-
Notifications
You must be signed in to change notification settings - Fork 132
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
[Fix #3721] Reducing group event size #2122
Conversation
cf8a7cf
to
4b6bf88
Compare
bf180f1
to
bd32fcb
Compare
PR job Reproducerbuild-chain build full_downstream -f 'https://raw.githubusercontent.com/${AUTHOR:apache}/incubator-kie-kogito-pipelines/${BRANCH:main}/.ci/buildchain-config-pr-cdb.yaml' -o 'bc' -p apache/incubator-kie-kogito-apps -u #2122 --skipParallelCheckout NOTE: To install the build-chain tool, please refer to https://github.com/kiegroup/github-action-build-chain#local-execution Please look here: https://ci-builds.apache.org/job/KIE/job/kogito/job/main/job/pullrequest_jobs/job/kogito-apps-pr/job/PR-2122/8/display/redirect Test results:
Those are the test failures: org.kie.kogito.index.ProcessDataIndexMongoDBIT.testProcessInstanceEvents1 expectation failed.JSON path data.UserTaskInstances[0].potentialGroups[0] doesn't match. Expected: managers Actual: null org.kie.kogito.jobs.service.messaging.InfinispanMessagingApiTest.createJobcreateJob() timed out after 10 minutesorg.kie.kogito.jobs.embedded.EmbeddedJobsServiceTest.testJobServicejava.util.ConcurrentModificationExceptionat java.base/java.util.ArrayList.forEach(ArrayList.java:1513) at java.base/java.util.Collections$UnmodifiableCollection.forEach(Collections.java:1092) at org.kie.kogito.jobs.embedded.EmbeddedJobsServiceTest.testJobService(EmbeddedJobsServiceTest.java:99) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:1013) at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:827) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) |
PR job Reproducerbuild-chain build full_downstream -f 'https://raw.githubusercontent.com/${AUTHOR:apache}/incubator-kie-kogito-pipelines/${BRANCH:main}/.ci/buildchain-config-pr-cdb.yaml' -o 'bc' -p apache/incubator-kie-kogito-apps -u #2122 --skipParallelCheckout NOTE: To install the build-chain tool, please refer to https://github.com/kiegroup/github-action-build-chain#local-execution Please look here: https://ci-builds.apache.org/job/KIE/job/kogito/job/main/job/pullrequest_jobs/job/kogito-apps-pr/job/PR-2122/9/display/redirect Test results:
Those are the test failures: org.kie.kogito.index.infinispan.ProcessDataIndexInfinispanKafkaIT.testProcessInstanceEvents1 expectation failed.JSON path errors doesn't match. Expected: null Actual: <[{message=Exception while fetching data (/UserTaskInstanceAttachmentUpdate) : Index 0 out of bounds for length 0, locations=[{line=1, column=12}], path=[UserTaskInstanceAttachmentUpdate], extensions={classification=DataFetchingException}}]> |
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.
Looks good to me, just a few comments and questions.
Awesome work @fjtirado !
return objectMapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false).registerModule(JsonFormat.getCloudEventJacksonModule()) | ||
.registerModule(new SimpleModule("Data Audit").addDeserializer(JobInstanceDataEvent.class, new JsonJobDataEventDeserializer())).findAndRegisterModules(); |
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.
Is it not needed to register JavaTimeModule anymore?
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.
findAndRegisterModules() automatically register all modules found in classpath
case MultipleProcessInstanceDataEvent.MULTIPLE_TYPE: | ||
MultipleProcessInstanceDataEvent dataEvent = buildEvent(cloudEvent, objectMapper, MultipleProcessInstanceDataEvent::new); | ||
if (cloudEvent.getData() != null) { | ||
String contentType = cloudEvent.getDataContentType(); |
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.
If it's not binary, it's assumming that it's "application/json", isn't it? But potentially it could be other even null
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.
if null, by cloudevent spec, application/json should be assumed.
case "ProcessInstanceErrorDataEvent": | ||
case MultipleProcessInstanceDataEvent.MULTIPLE_TYPE: | ||
MultipleProcessInstanceDataEvent dataEvent = buildEvent(cloudEvent, objectMapper, MultipleProcessInstanceDataEvent::new); | ||
if (cloudEvent.getData() != null) { |
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.
If data is null, you could add a warn logger to trace it in an else
clause, wdyt?:
LOGGER.warn("CloudEvent data is null for event of type: {}", cloudEvent.getType());
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.
hmmmm, It was not there before, I do not think is needed now.
PR job Reproducerbuild-chain build full_downstream -f 'https://raw.githubusercontent.com/${AUTHOR:apache}/incubator-kie-kogito-pipelines/${BRANCH:main}/.ci/buildchain-config-pr-cdb.yaml' -o 'bc' -p apache/incubator-kie-kogito-apps -u #2122 --skipParallelCheckout NOTE: To install the build-chain tool, please refer to https://github.com/kiegroup/github-action-build-chain#local-execution Please look here: https://ci-builds.apache.org/job/KIE/job/kogito/job/main/job/pullrequest_jobs/job/kogito-apps-pr/job/PR-2122/10/display/redirect Test results:
Those are the test failures: org.kie.kogito.jobs.embedded.EmbeddedJobsServiceTest.testJobServicejava.util.ConcurrentModificationExceptionat java.base/java.util.ArrayList.forEach(ArrayList.java:1513) at java.base/java.util.Collections$UnmodifiableCollection.forEach(Collections.java:1092) at org.kie.kogito.jobs.embedded.EmbeddedJobsServiceTest.testJobService(EmbeddedJobsServiceTest.java:99) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:1013) at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:827) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) |
PR job Reproducerbuild-chain build full_downstream -f 'https://raw.githubusercontent.com/${AUTHOR:apache}/incubator-kie-kogito-pipelines/${BRANCH:main}/.ci/buildchain-config-pr-cdb.yaml' -o 'bc' -p apache/incubator-kie-kogito-apps -u #2122 --skipParallelCheckout NOTE: To install the build-chain tool, please refer to https://github.com/kiegroup/github-action-build-chain#local-execution Please look here: https://ci-builds.apache.org/job/KIE/job/kogito/job/main/job/pullrequest_jobs/job/kogito-apps-pr/job/PR-2122/12/display/redirect Test results:
Those are the test failures: org.kie.kogito.jobs.embedded.EmbeddedJobsServiceTest.testJobServicejava.util.ConcurrentModificationExceptionat java.base/java.util.ArrayList.forEach(ArrayList.java:1513) at java.base/java.util.Collections$UnmodifiableCollection.forEach(Collections.java:1092) at org.kie.kogito.jobs.embedded.EmbeddedJobsServiceTest.testJobService(EmbeddedJobsServiceTest.java:99) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:1013) at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:827) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) |
Fix apache/incubator-kie-kogito-runtimes#3721
Merge wtih apache/incubator-kie-kogito-runtimes#3739