Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(client-java): support set variables when failing a job #10705

Merged
merged 2 commits into from
Oct 18, 2022
Merged

feat(client-java): support set variables when failing a job #10705

merged 2 commits into from
Oct 18, 2022

Conversation

skayliu
Copy link
Contributor

@skayliu skayliu commented Oct 13, 2022 •
edited

Description

Add support set variables when failing a job.

client.newFailCommand(...).variables(...)

Related issues

closes #9404

depends on #10701, #10702

Definition of Done

Not all items need to be done depending on the issue and the pull request.

Code changes:

  • The changes are backwards compatibility with previous versions
  • If it fixes a bug then PRs are created to backport the fix to the last two minor versions. You can trigger a backport by assigning labels (e.g. backport stable/1.3) to the PR, in case that fails you need to create backports manually.

Testing:

  • There are unit/integration tests that verify all acceptance criterias of the issue
  • New tests are written to ensure backwards compatibility with further versions
  • The behavior is tested manually
  • The change has been verified by a QA run
  • The impact of the changes is verified by a benchmark

Documentation:

  • The documentation is updated (e.g. BPMN reference, configuration, examples, get-started guides, etc.)
  • New content is added to the release announcement
  • If the PR changes how BPMN processes are validated (e.g. support new BPMN element) then the Camunda modeling team should be informed to adjust the BPMN linting.

Please refer to our review guidelines.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 13, 2022 •
edited

Test Results

   939 files  ±  0     939 suites  ±0   1h 45m 19s ⏱️ -30s
7 476 tests  - 66  7 469 ✔️  - 65  7 💤 ±0  0 ❌  - 1 
7 667 runs   - 65  7 658 ✔️  - 64  9 💤 ±0  0 ❌  - 1 

Results for commit 0965015. ± Comparison against base commit 0d9dba6.

♻️ This comment has been updated with latest results.

@saig0
Copy link
Member

saig0 commented Oct 14, 2022

@skayliu thank you for your contribution. 🎉 We'll have a look at your PR in the next few days. 👀

@korthout korthout self-requested a review October 14, 2022 14:43
korthout
korthout reviewed Oct 17, 2022
View reviewed changes
Copy link
Member

@korthout korthout left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @skayliu 🚀

👍 These changes look good, but they can only compile after #10702 is merged

I'll await with approval until we have a green CI on this PR.

@korthout
Copy link
Member

korthout commented Oct 18, 2022 •
edited

@skayliu You'll need to rebase this PR on top of main now that #10702 is merged. This should fix the compilation errors.

@skayliu skayliu force-pushed the 9404-fail-job-variables-support branch from dffa482 to 0965015 Compare October 18, 2022 09:21
@skayliu
Copy link
Contributor Author

skayliu commented Oct 18, 2022

@korthout, It's done.

github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 18, 2022
View reviewed changes
clients/java/src/main/java/io/camunda/zeebe/client/api/command/FailJobCommandStep1.java
* @return the builder for this command. Call {@link #send()} to complete the command and send
* it to the broker.
*/
FailJobCommandStep2 variables(Object variables);

Check notice

Code scanning / CodeQL

Confusing overloading of methods

Method FailJobCommandStep2.variables(..) could be confused with overloaded method [variables](1), since dispatch depends on static types. Method FailJobCommandStep2.variables(..) could be confused with overloaded method [variables](2), since dispatch depends on static types.
clients/java/src/main/java/io/camunda/zeebe/client/api/command/FailJobCommandStep1.java
* @return the builder for this command. Call {@link #send()} to complete the command and send
* it to the broker.
*/
FailJobCommandStep2 variables(InputStream variables);

Check notice

Code scanning / CodeQL

Confusing overloading of methods

Method FailJobCommandStep2.variables(..) could be confused with overloaded method [variables](1), since dispatch depends on static types.
korthout
korthout approved these changes Oct 18, 2022
View reviewed changes
Copy link
Member

@korthout korthout left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @skayliu LGTM 🥇

bors merge

@korthout korthout added the hacktoberfest-accepted Marks an issue as having been accepted as a Hacktoberfest contribution label Oct 18, 2022
@zeebe-bors-camunda
Copy link
Contributor

Build succeeded:

@zeebe-bors-camunda zeebe-bors-camunda bot merged commit ff72a85 into camunda:main Oct 18, 2022
@skayliu skayliu deleted the 9404-fail-job-variables-support branch October 18, 2022 14:38
@korthout korthout mentioned this pull request Oct 21, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@korthout korthout korthout approved these changes

Assignees

@skayliu skayliu

Labels
hacktoberfest-accepted Marks an issue as having been accepted as a Hacktoberfest contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

I can set variables when failing a job
3 participants
@skayliu @saig0 @korthout