-
Notifications
You must be signed in to change notification settings - Fork 209
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
[incubator-kie-issues-1517] Add Transactional Rest endpoints to UserTasks #3701
Conversation
|
||
/** | ||
* There is a preference order about how to compute transaction enabling. | ||
* 1. We check the property for the custom generator and |
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.
Comment should be finished to describe the logic which properties are taken into account.
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.
fixed
|
||
/** | ||
* There is a preference order about how to compute transaction enabling. | ||
* 1. We check the property for the custom generator and |
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.
Same as above (or just add the comment in one place)
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.
fixed
private List<Work> descriptors; | ||
private TemplatedGenerator producerTemplateGenerator; | ||
private TemplatedGenerator restTemplateGenerator; | ||
boolean transactionEnabled; |
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.
This field does not seem to be used anywhere and can be removed.
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.
fixed
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.
Question regarding UserTaskCodegen
. Running an example with kogito.processes.transactionEnabled=true
, I can see @Transactional
annotations in the following generated file:
@ApplicationScoped
public class UserTasksServiceProducer {
@Produces
@jakarta.transaction.Transactional()
public UserTaskService userTaskService(Application application) {
return new UserTaskServiceImpl(application);
}
What I was expecting to see instead was @Transactional
annotation added to UserTasksResource
:
@Path("/usertasks/instance")
public class UserTasksResource {
@GET
@Produces(MediaType.APPLICATION_JSON)
public List<UserTaskView> list(@QueryParam("user") String user, @QueryParam("group") List<String> groups) {
return userTaskService.list(IdentityProviders.of(user, groups));
}
Am I missing something?
Hi Martin, in theory should be the custom property kogito.usertasks.transactionEnabled=true or kogito.transactionEnabled=true or nothing at all |
@@ -61,7 +65,9 @@ public static String globalProperty(String propertyName) { | |||
* @see CodegenUtil#getProperty | |||
*/ | |||
public static boolean isTransactionEnabled(Generator generator, KogitoBuildContext context) { | |||
return getProperty(generator, context, TRANSACTION_ENABLED, Boolean::parseBoolean, true); | |||
boolean propertyValue = getProperty(generator, context, TRANSACTION_ENABLED, Boolean::parseBoolean, true); | |||
LOG.info("trying to compute property {} for generator {} property with value {}", generator.name(), TRANSACTION_ENABLED, propertyValue); |
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.
Just a nit-pick, generator.name()
and TRANSACTION_ENABLED
should be switched around
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.
fixed
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.
LGTM, thank you @elguardian
python failure realted to env |
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 #3701 --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-3701/3/display/redirect Test results:
Those are the test failures: |
Closes: apache/incubator-kie-issues#1517