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.

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

Document instance migration phases 3 and 4 #4348

Merged

Conversation

korthout
Copy link
Member

@korthout korthout commented Sep 25, 2024

Description

closes camunda/camunda#22680

When should this change go live?

  • This is a bug fix, security concern, or something that needs urgent release support.
  • This is already available but undocumented and should be released within a week.
  • This on a specific schedule and the assignee will coordinate a release with the DevEx team. (apply hold label or convert to draft PR)
  • This is part of a scheduled alpha or minor. (apply alpha or minor label)
  • There is no urgency with this change and can be released at any time.

WIP

PR Checklist

  • My changes are for an already released minor and are in /versioned_docs directory.
  • My changes are for the next minor and are in /docs directory (aka /next/).

WIP

korthout and others added 4 commits September 24, 2024 10:54
With 8.6, instance migration will have a few more error cases to ensure
process instances are always migrated correctly.
With 8.6, instance migration will have a few more limitations to ensure
process instances are always migrated correctly.
Boundary events can now be mapped in instance migrations. This results
in migrating the associated subscription.

While this change adjusts the limitations and notes that this mapping
was not supported, it does not yet provide an example to showcase the
behavior.
This is not finished yet, but adds a first section for dealing with
catch events.

Co-authored-by: berkaycanbc <124141078+berkaycanbc@users.noreply.github.com>
Copy link
Contributor

github-actions bot commented Sep 25, 2024

👋 🤖 🤔 Hello! Did you make your changes in all the right places?

These files were changed only in docs/. You might want to duplicate these changes in versioned_docs/version-8.5/.

  • docs/apis-tools/zeebe-api/gateway-service.md
  • docs/components/concepts/assets/process-instance-migration/migration-boundary-event_after.png
  • docs/components/concepts/assets/process-instance-migration/migration-catch-event-different-target-trigger-updated.png
  • docs/components/concepts/assets/process-instance-migration/migration-catch-event-different-target.png
  • docs/components/concepts/assets/process-instance-migration/migration-catch-event-identical-target-trigger-updated.png
  • docs/components/concepts/assets/process-instance-migration/migration-catch-event-source.png
  • docs/components/concepts/assets/process-instance-migration/migration-catch-event-target-added.png
  • docs/components/concepts/process-instance-migration.md

You may have done this intentionally, but we wanted to point it out in case you didn't. You can read more about the versioning within our docs in our documentation guidelines.

@berkaycanbc berkaycanbc changed the title Document instance migration phases 4 and 5 Document instance migration phases 3 and 4 Sep 27, 2024
korthout and others added 13 commits September 27, 2024 14:24
These headers should all be H3 under the H2 header for supported
elements. This ensures they're displayed as part of rather than their
own sections.
The concept page should focus on what users can achieve with this
feature (the concept) and how to use it. The limitations are in the
bottom so it doesn't distract from teaching the reader about the
concept. Which elements are supported is just further clarifying the
limitations and capabilities.
Move the headers so it's part of rather than separate sections.
@berkaycanbc berkaycanbc marked this pull request as ready for review October 1, 2024 14:11
@berkaycanbc berkaycanbc marked this pull request as draft October 1, 2024 14:12
@berkaycanbc berkaycanbc force-pushed the korthout-22680-document-instance-migration-phases-4-and-5 branch from b759a4d to 1a3a695 Compare October 1, 2024 14:22
Copy link
Member Author

@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 @berkaycanbc 🚀

👍 Lots of good changes. I've shared some suggestions to further improve this, but I think this is pretty solid.

❌ We should update the limitations section, e.g. it still mentions detaching boundary events instead of the more general catch events.

💭 Some additional thoughts:

  • I wonder if some of the sections are too wordy. Can we condense it a bit to avoid losing readers?
  • I wonder whether we need to clarify the internals a bit better near the start like the C7 docs do:
    • what do we actually do when migrating, i.e. change some properties like processDefinitionKey, bpmnProcessId, version, and elementId
    • what happens when a migration is rejected? (example for modification)

docs/components/concepts/process-instance-migration.md Outdated Show resolved Hide resolved
docs/components/concepts/process-instance-migration.md Outdated Show resolved Hide resolved
docs/components/concepts/process-instance-migration.md Outdated Show resolved Hide resolved
docs/components/concepts/process-instance-migration.md Outdated Show resolved Hide resolved
docs/components/concepts/process-instance-migration.md Outdated Show resolved Hide resolved
docs/components/concepts/process-instance-migration.md Outdated Show resolved Hide resolved
docs/components/concepts/process-instance-migration.md Outdated Show resolved Hide resolved
@@ -858,9 +858,11 @@ Returned if:
- Not all active elements in the given process instance are mapped to the elements in the target process definition
- A mapping instruction changes the type of an element or event
- A mapping instruction changes the implementation of a task
- A mapping instruction detaches a boundary event from an active element
Copy link
Member Author

Choose a reason for hiding this comment

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

❌ We still need adjust this to cover all catch events

Suggested change
- A mapping instruction detaches a boundary event from an active element
- A mapping instruction detaches a catch event from an active element

Copy link
Contributor

Choose a reason for hiding this comment

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

@korthout this limitation was outdated as I added an example after it. I refactored it to catch events in general but keep the example for boundary event. Please take another look. 👀

@berkaycanbc berkaycanbc force-pushed the korthout-22680-document-instance-migration-phases-4-and-5 branch from 9770415 to 43a94fb Compare October 4, 2024 09:50
@mesellings mesellings requested review from mesellings and removed request for a team October 4, 2024 09:51
@mesellings
Copy link
Contributor

@korthout I can take a look at this for you 👍

@camunda/tech-writers pinging you since this one needs to make it to 8.6 docs. Sorry for asking for a review with so little time left. 🙇‍♂️

Np @korthout I can take a look at this for you from a writing perspective 👍

@mesellings mesellings added the deploy Stand up a temporary docs site with this PR label Oct 4, 2024
@berkaycanbc berkaycanbc force-pushed the korthout-22680-document-instance-migration-phases-4-and-5 branch from 0f958c0 to 075dcc8 Compare October 4, 2024 10:09
@berkaycanbc
Copy link
Contributor

@mesellings, I just sent final commits to simplify things and re-organize limitations. Also, I added details about rejection cases to RPC docs.

@github-actions github-actions bot temporarily deployed to camunda-docs October 4, 2024 10:28 Destroyed
mesellings
mesellings previously approved these changes Oct 4, 2024
Copy link
Contributor

@mesellings mesellings left a comment

Choose a reason for hiding this comment

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

@korthout Lgtm - just some minor (non-blocking) suggestions and grammatical issues I spotted. I think we could split some of the long paragraphs into bulleted lists for readability/scanning, but that can be done at a later date. Great work 👍 🚀

docs/components/concepts/process-instance-migration.md Outdated Show resolved Hide resolved
docs/components/concepts/process-instance-migration.md Outdated Show resolved Hide resolved
docs/components/concepts/process-instance-migration.md Outdated Show resolved Hide resolved
docs/components/concepts/process-instance-migration.md Outdated Show resolved Hide resolved
docs/components/concepts/process-instance-migration.md Outdated Show resolved Hide resolved
docs/components/concepts/process-instance-migration.md Outdated Show resolved Hide resolved
docs/components/concepts/process-instance-migration.md Outdated Show resolved Hide resolved
docs/components/concepts/process-instance-migration.md Outdated Show resolved Hide resolved
docs/components/concepts/process-instance-migration.md Outdated Show resolved Hide resolved
docs/components/concepts/process-instance-migration.md Outdated Show resolved Hide resolved
@berkaycanbc
Copy link
Contributor

@korthout Lgtm - just some minor (non-blocking) suggestions and grammatical issues I spotted. I think we could split some of the long paragraphs into bulleted lists for readability/scanning, but that can be done at a later date. Great work 👍 🚀

Thanks for the review @mesellings! I applied all of your suggestions. 🚀

Could you please specify which paragraphs you would prefer to convert into bulleted lists?

@berkaycanbc berkaycanbc enabled auto-merge (squash) October 7, 2024 09:30
Copy link
Member Author

@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 for all the great improvements and changes @berkaycanbc

I had some final comments that I wanted to share

docs/apis-tools/zeebe-api/gateway-service.md Outdated Show resolved Hide resolved
docs/apis-tools/zeebe-api/gateway-service.md Outdated Show resolved Hide resolved
docs/apis-tools/zeebe-api/gateway-service.md Outdated Show resolved Hide resolved
docs/apis-tools/zeebe-api/gateway-service.md Outdated Show resolved Hide resolved
Copy link
Contributor

@mesellings mesellings left a comment

Choose a reason for hiding this comment

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

Lgtm - nice work! 👍

@berkaycanbc berkaycanbc merged commit abb0b45 into main Oct 7, 2024
6 of 7 checks passed
@berkaycanbc berkaycanbc deleted the korthout-22680-document-instance-migration-phases-4-and-5 branch October 7, 2024 13:57
Copy link
Contributor

github-actions bot commented Oct 7, 2024

🧹 Preview environment for this PR has been torn down.

christinaausley pushed a commit that referenced this pull request Oct 7, 2024
* docs: add new instance migration grpc error cases

With 8.6, instance migration will have a few more error cases to ensure
process instances are always migrated correctly.

* docs: add new instance migration limitations

With 8.6, instance migration will have a few more limitations to ensure
process instances are always migrated correctly.

* docs: lift no-mapped boundary events limitation

Boundary events can now be mapped in instance migrations. This results
in migrating the associated subscription.

While this change adjusts the limitations and notes that this mapping
was not supported, it does not yet provide an example to showcase the
behavior.

* wip: add section for dealing with catch events

This is not finished yet, but adds a first section for dealing with
catch events.

* docs: document migrating catch events

* docs: enrich the section for handling catch events

* docs: move the section for handling catch events up

* docs: add supported bpmn elements section

This section can also be moved to another page

* docs: change supported elements headers

These headers should all be H3 under the H2 header for supported
elements. This ensures they're displayed as part of rather than their
own sections.

* docs: move supported elements to limitations

The concept page should focus on what users can achieve with this
feature (the concept) and how to use it. The limitations are in the
bottom so it doesn't distract from teaching the reader about the
concept. Which elements are supported is just further clarifying the
limitations and capabilities.

* docs: move supported elements under limitations

Move the headers so it's part of rather than separate sections.

* docs: add first set of visuals for handling catch events

* docs: update image to the correct one

* docs: add more visuals for handling catch events

* docs: add more visuals for handling catch events

* docs: use event subprocess visual for add/remove catch event

* docs: fix links to the support bpmn elements

* docs: add link to REST API

* docs: update same message name tip

* docs: add example for boundary event detachment

* docs: simplify limitations section

* docs: add another catch event limitation

* docs: use existing bpmn symbol SVGs

* docs: move changing the process instance state discussion to intro

* docs: add supported bpmn elements to limitations section

* docs: remove old comment

* docs: remove excessive sentence

* docs: remove unnecessary section

* docs: emphasize catch event limitation in general

* docs: apply suggestions from code review

* docs: add limitations for multi-instance body

* docs: add operational semantics

* docs: clarify what happens if migration fails

* docs: simplify catch event behaviour explanations

* docs: simplify internal execution explanations

* docs: group limitations section

* docs: add more rejection cases to RPC docs

* docs: add message subscription limitation

* docs: apply suggestions from tech writer review

---------

Co-authored-by: Nico Korthout <korthout@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.6.0 October 2024 minor release component:zeebe Issues related with Zeebe project deploy Stand up a temporary docs site with this PR
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

Document Phase 3 and Phase 4
3 participants