Add support for DeployResourceCommand #301
Conversation
|
@korthout It was a bit of a challenge adding support for the |
remcowesterhoud
force-pushed
the
298_deploy_resource_command
branch
from
ac314a9 to
b80f3c2
Compare
There was a problem hiding this comment.
LGTM 👍
It was a bit of a challenge adding support for the DeployResourceCommand. Since you know all about this I'd like to see your opinion on it!
I understand the difficulty in that the value type of the records on the log stream didn't change, and the logic to decide what the response type is depended on the value type. I see no other solution than to change that logic. So your solution seems fine to me.
🔧 What I'm still missing is assertions for the deployed decisions and decisions requirements, but I expect that you want to postpone this. I think that's fine.
Apart from that, I have a few nitpicked suggestions.
Nice work altogether 👏
engine/src/test/java/io/camunda/zeebe/process/test/engine/GrpcToLogStreamGatewayTest.java
Outdated
Show resolved
Hide resolved
engine/src/main/java/io/camunda/zeebe/process/test/engine/GrpcResponseWriter.java
Outdated
Show resolved
Hide resolved
engine/src/main/java/io/camunda/zeebe/process/test/engine/GrpcResponseWriter.java
Outdated
Show resolved
Hide resolved
engine/src/main/java/io/camunda/zeebe/process/test/engine/GrpcResponseWriter.java
Outdated
Show resolved
Hide resolved
qa/abstracts/src/main/java/io/camunda/zeebe/process/test/qa/abstracts/util/Utilities.java
Outdated
Show resolved
Hide resolved
This test checks if all the gateway endpoints are implemented. Since gRPC does not provide an interface which can force this during compile time we are at risk that updating the Zeebe version could result in ZPT not supporting all endpoints that can be triggered from the client. To circumvent this problem this test was created to make sure we implement all the methods gRPC expects us to.
The response mapping has been moved from the GrpcResponseWriter to the GrpcToLogStreamGateway. The writer still contains the mapping functions, but they are called from the gateway. This change is necessary in order to support both the DeployProcessCommand as the DeployResourceCommand. In the writer we don't have any context about the request available. Therefore it is unknown which request we received, and also which response we should return. The mapping was based on the ValueType of the record which is the same for both of these commands. By letting the gateway decide which mapper should be used this problem has been resolved. At the point of entry of the request we will also decide which mapper should be used for the response. We nog longer need to decide the mapper base on the ValueType.
DeployProcessCommand is deprecated since Zeebe 8.0.0. In return a new command has been added to the client: DeployResourceCommand. This commit makes it possible to receive this command and return the appropriate response.
DeployCommand has been deprecated since Zeebe 8.0.0. We should use the new DeployResourceCommand as an alternative.
remcowesterhoud
force-pushed
the
298_deploy_resource_command
branch
from
b80f3c2 to
0c94eae
Compare
Yes, as of now only the absolute bare minimum has been done regarding DMN so ZPT wouldn't break. No specific assertions are added, nor are they planned. For now users can verify their DMN by checking the result variables. |
|
Backport failed for Please cherry-pick the changes locally. git fetch origin stable/8.0
git worktree add -d .worktree/backport-301-to-stable/8.0 origin/stable/8.0
cd .worktree/backport-301-to-stable/8.0
git checkout -b backport-301-to-stable/8.0
ancref=$(git merge-base 7fd433083f5a0eb017e8658c604e0c8812f5316e 45b29737242f3e9ef2ae3b80a885ebe5b924d14e)
git cherry-pick -x $ancref..45b29737242f3e9ef2ae3b80a885ebe5b924d14e |
Description
Since Zeebe 8.0.0 the
DeployCommandhas been deprecated and theDeployResourceCommandhas been added. This PR adds support for the new command and makes sure we are defaulting to this in our QA tests.A refactoring had to be done in order to support both the
DeployCommandand theDeployResourceCommand. The commit messages will provide more context on why this was necessary.Related issues
closes #274
closes #298
Definition of Done
Not all items need to be done depending on the issue and the pull request.
Code changes:
Testing:
Documentation: