-
Notifications
You must be signed in to change notification settings - Fork 213
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
[KOGITO-9811] Evaluating timeout duration as expression #3250
Conversation
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-runtimes -u #3250 --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-runtimes-pr/job/PR-3250/6/display/redirect Test results:
Those are the test failures: org.kie.kogito.quarkus.serverless.workflow.deployment.livereload.LiveReloadProcessorTest.testGrpc1 expectation failed.Expected status code <201> but was <500>. |
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-runtimes -u #3250 --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-runtimes-pr/job/PR-3250/7/display/redirect Test results:
Those are the test failures: org.kie.kogito.quarkus.dmn.KogitoDMNCodeCodestartIT.testBuild[Run project return status is zero]expected: 0 but was: 1 org.kie.kogito.quarkus.ServerlessWorkflowCodestartTest.buildAllProjectsForLocalUse[Run project return status is zero]expected: 0 but was: 1 |
@@ -70,7 +71,7 @@ | |||
} | |||
], | |||
"timeouts": { | |||
"eventTimeout": "PT5S" | |||
"eventTimeout": "$CONST.duration" |
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.
I think that when we introduce a new feature, the idea is not to modify an existing example and include the new feature there by modifying the existing stuff. I strongly suggest here to let the old example be, and create a new one to test the new feature.
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.
You are right in general (specially for existing examples), but in this particular test case (not an example, but an IT), there is not need to duplicate the flow because there are already other flows using hardcoded duration. In fact, the test want to ensure that using an expression in an existing timeout test does not break the test.
try { | ||
Duration.parse(value); | ||
} catch (DateTimeParseException e) { | ||
throw new IllegalArgumentException(message, e); |
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.
Not sure if we have a way to know if a value "looks like" an expression rather than a Duration directly and react consequently. But in this code here, if the expression is invalid, what users will see, is a DataTimeParseError, which might be wrong in case of an expression with errors.
IDK, if we have an expression like: "...value" , users will see a date time parse exception.
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.
We can improve the message to show "the provided timeout is neither a valid expression or a valid duration", wdyt?
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-runtimes -u #3250 --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-runtimes-pr/job/PR-3250/9/display/redirect Test results:
Those are the test failures: org.kie.kogito.quarkus.dmn.KogitoDMNCodeCodestartIT.testBuild[Run project return status is zero]expected: 0 but was: 1 org.kie.kogito.quarkus.ServerlessWorkflowCodestartTest.buildAllProjectsForLocalUse[Run project return status is zero]expected: 0 but was: 1 |
* [KOGITO-9811] Moving expression from ForEach to Process * [KOGITO-9811] Resolving timer on runtime * [KOGITO-9811] Improving message to include the expression possibility
Allow expressions in timeout durations for SWF.
This requires changes at engine side.