-
Notifications
You must be signed in to change notification settings - Fork 602
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
I can deploy a DMN decision #8531
Conversation
* add new records for DMN resources: DRG and decision * split the DRG record into a metadata part that does not contain the DMN binary resource * the metadata record will be used for the deployment record eventually instead of the DRG record itself to avoid that the deployment contains the resource twice
* extract interface for transforming a resource to support transformers for other resource types * fix test cases by adding bpmn file extension to resource name
* accept deployment with DMN resources * reject deployment if a DMN resource is invalid * write one DRG record for each DMN resource * write one decision record for each decision in a DRG
@korthout I'm sorry that the PR got so big 😅 But the changes are a bit less than it seems because of some refactorings and DMN files for tests. |
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.
Great work! A large PR, but of high quality 👏 LGTM
Please have a look at my notes and the CodeQL warning before merging.
...col/src/main/java/io/camunda/zeebe/protocol/record/value/deployment/DecisionRecordValue.java
Show resolved
Hide resolved
...main/java/io/camunda/zeebe/engine/processing/deployment/transform/DeploymentTransformer.java
Show resolved
Hide resolved
@Override | ||
public boolean isDuplicate() { | ||
return false; | ||
} |
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.
❓ Do we want to always consider these records duplicate? Or did you implement this as a first iteration, and we can implement duplication checks for it later?
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 will implement the duplication checks later with #8174.
However, the records will always be no duplicates. We write the records only for new versions.
But we will use the property for the new deployment command #8065. If a DMN is already deployed then it is added to the deployment response and marked as duplicate. Same as for processes.
I added the property already to avoid later changes in the records and ES templates.
bors merge |
Description
DecisionRecord
andDecisionRequirementsRecord
DECISION
andDECISION_REQUIREMENTS
DecisionIntent.CREATED
andDecisionRequirementsIntent.CREATED
duplicate
property for the metadata and to align with the process recordDeploymentTransformer
to allow more than BPMN resources.bpmn
file extension to the resource name on deployment.bpmn
file extension (e.g. by using the Camunda modeler), this behavior change should not affect usersRelated issues
closes #8067
Definition of Done
Not all items need to be done depending on the issue and the pull request.
Code changes:
backport stable/0.25
) to the PR, in case that fails you need to create backports manually.Testing:
Documentation: