camunda / camunda-bpm-platform

Flexible framework for workflow and decision automation with BPMN and DMN. Integration with Quarkus, Spring, Spring Boot, CDI.
https://camunda.com/
Apache License 2.0
4.13k stars 1.56k forks source link

Persist task id in ACT_HI_VARINST table for task variables #4578

Open jyotisahu9 opened 2 months ago

jyotisahu9 commented 2 months ago

User Story (Required on creation)

As a Operations engineer, I would like to know all variables related to specific task

Functional Requirements (Required before implementation)

Technical Requirements (Required before implementation)

psavidis commented 2 months ago

Hi @jyotisahu9,

Could you explain further the Technical Requirements section?

It is not clear to me what is described there as well how it ties to the functional requirements section (being able to query all tasks variables from runtime and history tables)

jyotisahu9 commented 2 months ago

Hi @psavidis,

There is TASKID column in ACT_HI_VARINST table that remains null for task variable ( created through Execution listeners or task listeners). We would like to reuse this column to save task id to identify these variables related to specific task and query them.

There is VARSCOPE column in ACT_RU_VARIABLE table that persists the task id or process instance id to identify specific variable, but once that task is completed, entries are deleted from table.

Hope this clarifies the requirement. Let me know !!

Thanks, Jyoti

jyotisahu9 commented 2 months ago

Hi @psavidis ,

Does this looks good to you? Do you have any questions?

Thanks, Jyoti

psavidis commented 1 month ago

Hello @jyotisahu9, I'm currently occupied on items of high priority for the upcoming release.

I think i understood the requirement, the description / formatting looked a bit confusing to me.

In the meantime, could you break down the technical description into a list of high-level steps that describe what needs to be added? You could for example break it down into layers and mention the table (as you did), the query, the service involved, rest-api etc.

I will review it once i have time and make any necessary adjustments if needed. The goal is to have a technical description which reflects on a high-level scale what needs to be done before starting to work.

Are you going to work on this item?

jyotisahu9 commented 1 month ago

Hi @psavidis

Yes, I will be working on this task.

I would be making the changes to persist taskId in ACT_HI_VARINST table for following two kind of variables: **1. Task Input variables

  1. Task Listener variables**

Whenever a task element executes( be it through process instance or standalone task), following steps takes place:

  1. createHistoricVariableEvent - Task input variables are created
  2. createActivityInstanceStartEvt
  3. createTaskInstanceCreateEvt
  4. createActivityInstanceUpdateEvt
  5. createTaskInstanceUpdateEvt - Task listener variables are created

I would be plugging in code to set taskId for HistoricVariableInstanceEntity at below steps:

  1. createHistoricVariableEvent - Task input variables are created, but taskId is not generated.
  2. createActivityInstanceStartEvt
  3. createTaskInstanceCreateEvt - Task Id is created at this step
  4. createActivityInstanceUpdateEvt - As all the HistoricVariableInstanceEntities created in step1 are in cache at this point, retrieve them from cache, update taskId for all HistoricVariableInstanceEntities whose activityInstanceId matches with current execution activityInstanceId of task.
  5. createTaskInstanceUpdateEvt - Task listener variables are created, task id is already created at this point so set the task id for HistoricVariableInstanceEntity.

API to start task are:

  1. Through process instance is : /process-definition/key/{key}/start
  2. As a Standalone task : /task/create

API to retrieve task historic variables where taskId will be returned as response parameter are: Get Variable Instances : /history/variable-instance Get Variable Instance : /history/variable-instance/{id}

jyotisahu9 commented 1 month ago

Hi @psavidis,

Request you to go through details. Also this is more of bug than feature, I initially created it as feature but then could not change the category.

I have my PR ready that can be pushed to understand the whole implementation.

Thanks, Jyoti

jyotisahu9 commented 3 weeks ago

Hi @psavidis ,

Could you go over the pull request and issue description when you get a chance.

Thanks, Jyoti

psavidis commented 1 week ago

Hello @jyotisahu9 , Apologies for the late response, it's been quite busy lately.

I'll try to find some time during this week and review the ticket and PR properly.

Till then, a question: Why do you think its a bug?

Best, Petros

jyotisahu9 commented 1 week ago

Hi @psavidis,

Thanks for your response.

I think its a bug because there is TASKID column in ACT_HI_VARINST table but it remains null for task variable. This table saves all variable information and shall persist task id to identify specific task variable

Best, Jyoti

psavidis commented 2 days ago

Hello @jyotisahu9 ,

I had a look at the feature request and your pull request and i'd like to share with you my scepticism and questions.

So far, i'm not entirely sure what is the intent of this feature request. You outlined in a elaborate way what you're going to change but didn't specify why.

I It seems to me from the pull request of your contribution that there is some confusion about the concept of variables and taskId.

Observation

I've executed the testTaskIdInHistoricVariableInstance (ref) from the PR and the test passes on master.

Documentation

If you read the official documentation around process variables, you'll notice the variables can have different scope executions. Some of these executions concern process-instances and others are related to tasks.

Using that as a reference, i'd like to understand better:

a) What is the problem you'd like to solve b) Why is that a problem

Best, Petros