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
Feature/git issue 4049 process instance api enhancement #4222
base: master
Are you sure you want to change the base?
Feature/git issue 4049 process instance api enhancement #4222
Conversation
…ent with Process definition Key attribute Feat(engine) : Process Instance API Enhancement with Process definition Key attribute -Enhance the GET Process Instance based on PID /Get Process Instance List /Post Process instance list to include Process definition key in the API response -Enhance Swagger specs related to GIT Issue : 4049 Signed-off-by: Shenoy, Nandan <Nandan.Shenoy@fmr.com>
…nhancement with Process definition Key attribute Feat(engine) : Process Instance API Enhancement with Process definition Key attribute -Enhance the GET Process Instance based on PID /Get Process Instance List /Post Process instance list to include Process definition key in the API response -Enhance Swagger specs related to GIT Issue : 4049 Signed-off-by: Shenoy, Nandan <Nandan.Shenoy@fmr.com>
…nhancement with Process definition Key attribute Feat(engine) : Process Instance API Enhancement with Process definition Key attribute -Enhance the GET Process Instance based on PID /Get Process Instance List /Post Process instance list to include Process definition key in the API response -Enhance Swagger specs related to GIT Issue : 4049 Signed-off-by: Shenoy, Nandan <Nandan.Shenoy@fmr.com>
…ss Instance API Enhancement with Process definition Key attribute Feat(engine) : Process Instance API Enhancement with Process definition Key attribute -Enhance the GET Process Instance based on PID /Get Process Instance List /Post Process instance list to include Process definition key in the API response -Enhance Swagger specs related to GIT Issue : 4049 Signed-off-by: Shenoy, Nandan <Nandan.Shenoy@fmr.com>
…I Enhancement with Process definition Key attribute Feat(engine) : Process Instance API Enhancement with Process definition Key attribute -Enhance the GET Process Instance based on PID /Get Process Instance List /Post Process instance list to include Process definition key in the API response -Enhance Swagger specs related to GIT Issue : 4049 Signed-off-by: Shenoy, Nandan <Nandan.Shenoy@fmr.com>
… Enhancement with Process definition Key attribute Feat(engine) : Process Instance API Enhancement with Process definition Key attribute -Enhance the GET Process Instance based on PID /Get Process Instance List /Post Process instance list to include Process definition key in the API response -Enhance Swagger specs related to GIT Issue : 4049 Signed-off-by: Shenoy, Nandan <Nandan.Shenoy@fmr.com>
…th Process definition Key attribute Feat(engine) : Process Instance API Enhancement with Process definition Key attribute -Enhance the GET Process Instance based on PID /Get Process Instance List /Post Process instance list to include Process definition key in the API response -Enhance Swagger specs related to GIT Issue : 4049 Signed-off-by: Shenoy, Nandan <Nandan.Shenoy@fmr.com>
…th Process definition Key attribute Feat(engine) : Process Instance API Enhancement with Process definition Key attribute -Enhance the GET Process Instance based on PID /Get Process Instance List /Post Process instance list to include Process definition key in the API response -Enhance Swagger specs related to GIT Issue : 4049 Signed-off-by: Shenoy, Nandan <Nandan.Shenoy@fmr.com>
…ith Process definition Key attribute Feat(engine) : Process Instance API Enhancement with Process definition Key attribute -Enhance the GET Process Instance based on PID /Get Process Instance List /Post Process instance list to include Process definition key in the API response -Enhance Swagger specs related to GIT Issue : 4049 Signed-off-by: Shenoy, Nandan <Nandan.Shenoy@fmr.com>
…049 Process Instance API Enhancement with Process definition Key attribute Feat(engine) : Process Instance API Enhancement with Process definition Key attribute -Enhance the GET Process Instance based on PID /Get Process Instance List /Post Process instance list to include Process definition key in the API response -Enhance Swagger specs related to GIT Issue : 4049 Signed-off-by: Shenoy, Nandan <Nandan.Shenoy@fmr.com>
Hi @Nandanrshenoy, Thank you for raising this. Best, |
@yanavasileva, Thanks and Regards, |
Hi @Nandanrshenoy,
We see that you have signed the CLA on 26th of March but we are not sure what is the issue. I see that the PR is created with account @Nandanrshenoy but the commits are pushed with account @Nandan-shenoy. |
@yanavasileva , Regards, |
@yanavasileva , Regards, |
I am looking into the code. I need do some research before make up my mind. (Such as: are we concerned for performance as the REST API will do additional query, do we want to expose the field to the Java API (pd id is not)) And I need to read a bit for the |
Note to myself: Details
|
@yanavasileva , Thanks and Regards, |
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.
Hi @Nandanrshenoy,
I evaluated the proposed changes.
❌My suggestion is to add a new process definition key column to the Execution table instead of relying on an association and having additional queries to the database. So similar to processDefintionId
, persist the processDefinitionKey
as well. We already have the same in other tables (job, job def).
🔧 Please remove the GIT Issue
comments from the code.
🔧 I found the following test cases that can be extended with the new API:
- https://github.com/camunda/camunda-bpm-platform/blob/master/engine/src/test/java/org/camunda/bpm/engine/test/standalone/entity/ExecutionEntityTest.java#L110-L111
- https://github.com/camunda/camunda-bpm-platform/blob/master/engine/src/test/java/org/camunda/bpm/engine/test/api/runtime/ExecutionQueryTest.java#L96-L101
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.
❌ Instead of using association
extend the table with PROC_DEF_KEY_
column. You will need to add the change to the create and update sql scripts.
public String getDefinitionKey() { | ||
return definitionKey; |
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.
🔧 Formatting:
public String getDefinitionKey() { | |
return definitionKey; | |
public String getDefinitionKey() { | |
return definitionKey; |
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.
❌ Failing test (the mocks are not updated): ProcessInstanceRestServiceInteractionTest.testGetSingleInstance
with
java.lang.AssertionError:
1 expectation failed.
JSON path definitionKey doesn't match.
Expected: aKey
Actual: null
at org.camunda.bpm.engine.rest.ProcessInstanceRestServiceInteractionTest.testGetSingleInstance(ProcessInstanceRestServiceInteractionTest.java:1090)
How to run the REST API tests (after the engine and sql-script modules are built)?:
mvn clean install -Pjersey2 -f engine-rest/engine-rest/pom.xml
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.
👍 Changes in OpenAPI look good.
Hi @yanavasileva , Thanks and Regards, |
Hi @Nandanrshenoy, Yes, you're right, the key is somehow static. (That's why it was not already in the execution table.)
Considering above, my suggestion is still the same to introduce a new column to the execution table. Best, |
Thanks @yanavasileva for sharing your point of view. Thanks and Regards, |
My suggestion is add new column in the execution table and add new result to the |
Feature Description:
The Get Process Instance API does not return the Process Definition Key in the API response.
How it is currently being handled
To fetch the process definition key associated with a process instance, the end user needs to first make a call to the Get Process Instance API through which the definition ID can be retrieved and then subsequently make a call the Get Process definition API(https://docs.camunda.org/rest/camunda-bpm-platform/7.21-SNAPSHOT/{url}/process-definition/{id}) to retrieve the definition key.
Advantage of having this feature
This feature if added will avert making the additional call and the retrieval of process definition key can be supported in a single call using Get Process Instance API.
Test Screenshot
Ticket: #4049