ci: build Tasklist and Operate frontends in distribution module #18279
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
houssain-barouni
commented
May 6, 2024
| @@ -20,7 +20,7 @@ jobs: | |||
| - version: 0.8.2-alpha1 | |||
| latest: false | |||
| with: | |||
| releaseBranch: main | |||
There was a problem hiding this comment.
needed to test with dry-run, I will revert it
There was a problem hiding this comment.
You could also use the other workflow https://github.com/camunda/zeebe/actions/workflows/camunda-platform-release-manual.yml and mark it as dryrun :)
houssain-barouni
added
component/operate
Zelldon
reviewed
May 7, 2024
| @@ -181,7 +181,7 @@ jobs: | |||
| -DlocalCheckout=${{ inputs.dryRun }} \ | |||
| -DskipQaBuild=true \ | |||
| -P-autoFormat \ | |||
| -Darguments='-P-autoFormat -DskipQaBuild=true -DskipChecks=true -DskipTests=true -Dskip.fe.build=false -Dspotless.apply.skip=false -Dskip.central.release=${SKIP_REPO_DEPLOY} -Dskip.camunda.release=${SKIP_REPO_DEPLOY} -Dzbctl.force -Dzbctl.rootDir=${ZBCTL_ROOT_DIR} -Dgpg.passphrase="${{ steps.secrets.outputs.MAVEN_CENTRAL_GPG_SIGNING_KEY_PASSPHRASE }}"' | |||
| -Darguments='-T0.5C -P buildDistFrontend,skipFrontendBuild,-autoFormat -DskipQaBuild=true -DskipChecks=true -DskipTests=true -Dspotless.apply.skip=false -Dskip.central.release=${SKIP_REPO_DEPLOY} -Dskip.camunda.release=${SKIP_REPO_DEPLOY} -Dzbctl.force -Dzbctl.rootDir=${ZBCTL_ROOT_DIR} -Dgpg.passphrase="${{ steps.secrets.outputs.MAVEN_CENTRAL_GPG_SIGNING_KEY_PASSPHRASE }}"' | |||
There was a problem hiding this comment.
❌ That is a bit confusing :/
Could we either rename the profiles skipFrontendBuild or add a comment here why we doing this. See above I tried to document a bit the used parameter, might be useful also to explain why we have to use the two settings.
SetVariable endpoint is making changes on the state of the process instance. Therefore, it is not idempotent. If we retry setting variables in different stages of process instance, it might provide different behaviour. Because of that we shouldn't retry SetVariables on timeouts (DEADLINE_EXCEEDED) where the processing of the engine can continue, so the process instance state changes. For UNAVAILABLE and RESOURCE_EXHAUSTED, the broker will not accept further commands, so that when we retry the SetVariables command, the process instance state will stay the same.
Modifies the Atomix utility `Version` to use the more complete `SemanticVersion` utility from Zeebe. This fully complies with the semantic version spec, and is much more lenient about adding build-metadata and so on. At the same time, we preserve the check on start up that the version is parse-able at all, to avoid issues where we would run with un-parseable versions and potentially use a snapshot which could cause inconsistencies.
Separating the release version and release branch and expecting as input, allows to create release candidates, as this is normally done from an release branch that has not the same name as the release version. For example: branch = 'release-8.6.0-alpha1' and version = 'release-8.6.0-alpha1-rc1'
Readers are more likely to look for this behavior class in the distribution package than in the common package. I noticed that it wasn't there when I was looking for it myself. I think it makes sense to keep the functionality close together, so I've placed it next to the CommandRedistributor and the CommandDistributionAcknowledgeProcessor.
I want to refactor the CommandDistributionBehavior. So, I started to add some additional JavaDoc to the class/methods to explain what it does and how it works. This pushed me to re-read our docs entry for generalized_distribution. While reading, I saw some opportunities to improve the text: - clarified why we need generalized_distribution - clarified that CommandDistribution records only exist on the distributing partition - added a tip to use DistributedTypedRecordProcessor Then I came back to the JavaDoc of the CommandDistributionBehavior: - linked to the generalized_distribution docs - clarified that the distributeCommand method distributes to all other partitions. - clarified what the distributionKey is used for as this key is very important to the feature. It must encode the distributing partition, and must be unique.
Distributed commands are always acknowledged using the distributionKey, which is always the command key. We can simply inline this parameter as there is no option to provide a different key.
It makes sense that the distribution key encodes the partition id. So, it's better if we use this local variable name because it just makes things more clear.
Simple local variable extraction to reduce duplication
The value of command.getValueType is equivalent to the local variable valueType.
The intent from the distribution record is equivalent to the intent of the command. This change allows to no longer need the command in the distributeToPartition parameters.
I've added a comment explaining that this is necessary, but that we don't fully grasp why. I've also moved it closer to the usage
Records are expensive to construct. So, if we can re-use them we should
Adds a new public method to distribute a command to a subset of partitions, rather than to all other partitions.
Adds unit tests for the distributeCommand methods of the CommandDistributionBehavior class. I've explicitly chosen not to test this by adding special deployment tests, because I don't think that makes sense. This behavior can be used for different commands, not just deployment records. Instead of spinning up a test engine, this is a must more focused test that verifies that records are added to the processing result builder and that there are interactions with the InterPartitionCommandSender. To support this, I've introduced a new FakeProcessingResultBuilder that can be used in such tests.
It was pointed out in a review, that this documentation was not clear about how the communication is unreliable. This is essential to understand the need for command distribution as a way to add reliable communication between the partitions.
It was pointed out that the second sentence about the different purposes of the distribution key was hard to read. Co-authored-by: berkaycanbc <berkay.can@camunda.com>
Records are expensive to construct. So, if we can re-use them we should
While not impacting the behavior right now, resetting records may help avoid unforseen bugs when we make changes later.
… build-fe-dist
github-actions
bot
added
the
component/identity
|
handled in #18483 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This is to fix the issues found when running
release:performwith concurrency, yarn install could not be run at the same time for Operate and Tasklist, thus we had to disable the concurrency for the releasing (which make it taking ~6 minutes longer)In this pull request,
frontend-maven-pluginexecutions are run bydistmodule to ensure that they are not run in parallel, it also allows thatinstall-node-and-yarngoal is excuted only once.A new profile
buildDistFrontendwas created to be used for mvnrelease:performalong withskipFrontendBuildthat skips the same plugin executions in client submodules in Tasklist and OperateThe plugin is kept though in client modules, to allow developers to start the applications using webapp only (without building and running the single JAR)
Dry-run for this branch https://github.com/camunda/zeebe/actions/runs/8971123064
Related issues
closes #