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

Escalate errors in output mapping evaluation #9668

Merged
merged 3 commits into from
Jul 5, 2022
Merged

Escalate errors in output mapping evaluation #9668

merged 3 commits into from
Jul 5, 2022

Conversation

pihme
Copy link
Contributor

@pihme pihme commented Jul 1, 2022

Description

Changes the expression which merges the result of the output mapping to be consistent

Review Hints

There are two versions of the expression. The last one is easier to comprehend, in my mind. If you like the first one better, we can just drop the last commit.

Related issues

closes #9543

Definition of Done

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.

@pihme pihme requested a review from saig0 July 1, 2022 13:11
@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2022 •
edited

Unit Test Results

   784 files  ±0     784 suites  ±0   1h 32m 21s ⏱️ +4s
5 528 tests +8  5 521 ✔️ +8  7 💤 ±0  0 ❌ ±0 
5 697 runs  +5  5 690 ✔️ +5  7 💤 ±0  0 ❌ ±0 

Results for commit c769765. ± Comparison against base commit b51f397.

♻️ This comment has been updated with latest results.

@pihme pihme force-pushed the 9543-escaöate-errors-in-output-mapping-evaluation branch from 5bb3e57 to c769765 Compare July 1, 2022 13:40
saig0
saig0 approved these changes Jul 4, 2022
View reviewed changes
Copy link
Member

@saig0 saig0 left a comment

Choose a reason for hiding this comment

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

@pihme great job 🎉 I really like your idea to fix the behavior 👍

I prefer the second option. I have only one minor suggestion for it. Please have a look at my comment.

...camunda/zeebe/engine/processing/deployment/model/transformer/VariableMappingTransformer.java Outdated Show resolved Hide resolved
...o/camunda/zeebe/engine/processing/variable/mapping/VariableOutputMappingTransformerTest.java Show resolved Hide resolved
@saig0 saig0 changed the title 9543 escalate errors in output mapping evaluation Escalate errors in output mapping evaluation Jul 4, 2022
@pihme
refactor(engine): flip logic in expression
da446e7 
The expression will appear in the failure message of the evaluation.
@pihme pihme force-pushed the 9543-escaöate-errors-in-output-mapping-evaluation branch from c769765 to da446e7 Compare July 5, 2022 10:04
@pihme
Copy link
Contributor Author

pihme commented Jul 5, 2022

bors merge

@zeebe-bors-camunda
Copy link
Contributor

Build succeeded:

@zeebe-bors-camunda zeebe-bors-camunda bot merged commit 7f2827a into main Jul 5, 2022
@zeebe-bors-camunda zeebe-bors-camunda bot deleted the 9543-escaöate-errors-in-output-mapping-evaluation branch July 5, 2022 10:27
@backport-action backport-action mentioned this pull request Jul 5, 2022
Merged
@backport-action
Copy link
Collaborator

Successfully created backport PR #9695 for stable/1.3.

@backport-action backport-action mentioned this pull request Jul 5, 2022
Merged
@backport-action
Copy link
Collaborator

Successfully created backport PR #9696 for stable/8.0.

zeebe-bors-camunda bot added a commit that referenced this pull request Jul 5, 2022
@zeebe-bors-camunda @pihme
merge: #9696
6a4ccce 
9696: [Backport stable/8.0] Escalate errors in output mapping evaluation r=pihme a=backport-action

# Description
Backport of #9668 to `stable/8.0`.

relates to #9543

Co-authored-by: pihme <pihme@users.noreply.github.com>
zeebe-bors-camunda bot added a commit that referenced this pull request Jul 5, 2022
@zeebe-bors-camunda @pihme
merge: #9695
1f3cdc8 
9695: [Backport stable/1.3] Escalate errors in output mapping evaluation r=pihme a=backport-action

# Description
Backport of #9668 to `stable/1.3`.

relates to #9543

Co-authored-by: pihme <pihme@users.noreply.github.com>
zeebe-bors-camunda bot added a commit that referenced this pull request Jul 5, 2022
@zeebe-bors-camunda @pihme
merge: #9696
663ba9f 
9696: [Backport stable/8.0] Escalate errors in output mapping evaluation r=pihme a=backport-action

# Description
Backport of #9668 to `stable/8.0`.

relates to #9543

Co-authored-by: pihme <pihme@users.noreply.github.com>
zeebe-bors-camunda bot added a commit that referenced this pull request Jul 5, 2022
@zeebe-bors-camunda @pihme
merge: #9695
fdb86b2 
9695: [Backport stable/1.3] Escalate errors in output mapping evaluation r=pihme a=backport-action

# Description
Backport of #9668 to `stable/1.3`.

relates to #9543

Co-authored-by: pihme <pihme@users.noreply.github.com>
zeebe-bors-camunda bot added a commit that referenced this pull request Jul 5, 2022
@zeebe-bors-camunda @pihme
merge: #9696
ac21469 
9696: [Backport stable/8.0] Escalate errors in output mapping evaluation r=pihme a=backport-action

# Description
Backport of #9668 to `stable/8.0`.

relates to #9543

Co-authored-by: pihme <pihme@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saig0 saig0 saig0 approved these changes

Assignees
No one assigned
Labels
version:1.3.13
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Output mapping doesn't create an incident if a variable is missing
4 participants
@pihme @backport-action @saig0 @npepinpe