Skip to content

Improve documentation of workflows and its steps #1020

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

tjeerddie
Copy link
Contributor

@tjeerddie tjeerddie commented Jul 31, 2025

  • changed architecture workflows to explain everything without code, only referencing with links to docs with code.
  • add a page in getting started for workflows to explain how to create workflows, moving old docs from the architecture and beginner workshop.
  • add workflows page with the different types of workflow util functions in refence-docs/workflows folder.
  • add re-usable step and singledispatch code examples in refence-docs/workflows/workflow-steps

Todo's:

  • BCP in workflows: Example of “Reusable” step pattern (fn that returns step)
  • BCP in workflows: No commits allowed inside a step: Commit is disabled on the ORM (This is disabled in the orch-core)
  • BCP in workflows: Steps are Retryable - should be idempotent
  • BCP in workflows: One create workflow per product
  • BCP in workflows: Many modify workflows (But maybe “few”?)
  • BCP in workflows: One terminate
  • BCP in workflows: One validate
  • BCP in workflows: Single-dispatch pattern
  • Workflows are an audittrail, not a transporter of data
  • Workflow lifecyclemapping: Constrain workflow usage per lifecycle WF_USABLE_MAPPING

Related: #533

@tjeerddie tjeerddie marked this pull request as draft July 31, 2025 16:22
@tjeerddie tjeerddie force-pushed the improve-workflow-documentation branch from 781b224 to f229204 Compare August 4, 2025 12:26
@tjeerddie tjeerddie marked this pull request as ready for review August 4, 2025 13:35
Copy link

codspeed-hq bot commented Aug 4, 2025

CodSpeed Performance Report

Merging #1020 will not alter performance

Comparing improve-workflow-documentation (3e983fc) with main (1818a0d)

Summary

✅ 13 untouched benchmarks

Copy link

codecov bot commented Aug 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.93%. Comparing base (1818a0d) to head (3e983fc).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1020   +/-   ##
=======================================
  Coverage   84.93%   84.93%           
=======================================
  Files         214      214           
  Lines       10408    10408           
  Branches     1020     1020           
=======================================
  Hits         8840     8840           
  Misses       1295     1295           
  Partials      273      273           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@Mark90 Mark90 left a comment

Choose a reason for hiding this comment

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

Nice work! Read most of it and have left a bunch of suggestions you can apply (or not) and a few questions

@tjeerddie tjeerddie force-pushed the improve-workflow-documentation branch 2 times, most recently from 1ef1a60 to afbd2e4 Compare August 7, 2025 12:17
@tjeerddie tjeerddie force-pushed the improve-workflow-documentation branch from afbd2e4 to 436ef48 Compare August 7, 2025 12:25
- add workflow decorator to the workflow page in reference docs.
- update workflow-introduction to reference the workflow architecture instead of having copy pasted text.
- add empty line between headers and text.
- use link definitions for long urls.
@tjeerddie tjeerddie force-pushed the improve-workflow-documentation branch from 436ef48 to 3e983fc Compare August 7, 2025 12:40
- [terminate_workflow]
- [validate_workflow]

under the hood they all use a [workflow] decorator which can be used for tasks that don't fit any of the types above.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
under the hood they all use a [workflow] decorator which can be used for tasks that don't fit any of the types above.
Under the hood they all use a [workflow] decorator which can be used for tasks that don't fit any of the types above.


!!! example

for inspiration look at an example implementation of the [lazy workflow instances]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for inspiration look at an example implementation of the [lazy workflow instances]
For inspiration look at an example implementation of the [lazy workflow instances].


For `@modify_workflow`, `@validate_workflow` and `@terminate_workflow` the `subscription` is directly usable from the first step.

[Information about all usable step decorators can be found here](../architecture/application/workflow#workflow-steps)
Copy link
Contributor

Choose a reason for hiding this comment

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

Opinionized nit :) When the entire sentence is a link it becomes more difficult to spot - at least when using a light theme. The contrast between normal and clickable text helps it stand out for me.

Suggested change
[Information about all usable step decorators can be found here](../architecture/application/workflow#workflow-steps)
Information about all usable step decorators can be found on [the architecture page on workflows](../architecture/application/workflow#workflow-steps).

Create a new empty database migration with the following command:

```shell
PYTHONPATH=. python main.py db revision --head data --message "add User and UserGroup workflows"
Copy link
Contributor

@Mark90 Mark90 Aug 8, 2025

Choose a reason for hiding this comment

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

Think PYTHONPATH=. is a remnant from the past? I haven't had to use this in our own nor the example orchestrator. Imo it'd be better to have this in a troubleshooting section (if people indeed run into it) rather than prefixing it everywhere

Suggested change
PYTHONPATH=. python main.py db revision --head data --message "add User and UserGroup workflows"
python main.py db revision --head data --message "add User and UserGroup workflows"

Run the migration with the following command:

```shell
PYTHONPATH=. python main.py db upgrade heads
Copy link
Contributor

Choose a reason for hiding this comment

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

Same suggestion as above

Suggested change
PYTHONPATH=. python main.py db upgrade heads
python main.py db upgrade heads

return {"response": response}
```

While this approach works, the switch logic (via match or if isinstance) can become unwieldy as more product types are introduced.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
While this approach works, the switch logic (via match or if isinstance) can become unwieldy as more product types are introduced.
While this approach works, the switch logic (via `match` or `if isinstance()`) can become unwieldy as more product types are introduced.

Copy link
Contributor

@Mark90 Mark90 left a comment

Choose a reason for hiding this comment

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

Left a few small remaining comments.

Amazing work! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants