feat: Implement new partial execution logic for acyclic workflows (no-changelog) #10256
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
despairblue
changed the title
n8n-assistant
bot
added
core
despairblue
force-pushed
the
pay-1727-spike-partial-execution-logic
branch
from
a916157
to
55b3264
Compare
despairblue
force-pushed
the
pay-1727-spike-partial-execution-logic
branch
from
55b3264
to
fe50536
Compare
PARTIAL_EXECUTION_VERSION_DEFAULT can be 0 or 1
despairblue
force-pushed
the
pay-1727-spike-partial-execution-logic
branch
2 times, most recently
from
91a3552
to
37f5e28
Compare
despairblue
force-pushed
the
pay-1727-spike-partial-execution-logic
branch
from
37f5e28
to
320eeb7
Compare
4 tasks
despairblue
force-pushed
the
pay-1727-spike-partial-execution-logic
branch
from
5b30dfc
to
11685f5
Compare
tomi
previously approved these changes
Sep 18, 2024
Comment on lines
+51
to
+61
| //function findAllPinnedActivators(workflow: Workflow, pinData?: IPinData) { | ||
| // return Object.values(workflow.nodes) | ||
| // .filter( | ||
| // (node) => | ||
| // !node.disabled && | ||
| // pinData?.[node.name] && | ||
| // ['trigger', 'webhook'].some((suffix) => node.type.toLowerCase().endsWith(suffix)) && | ||
| // node.type !== 'n8n-nodes-base.respondToWebhook', | ||
| // ) | ||
| // .sort((a) => (a.type.endsWith('webhook') ? -1 : 1)); | ||
| //} |
There was a problem hiding this comment.
I'll remove this in a future PR. It's referenced in the todo above, so I don't forget:
|
|
n8n
|
||||||||||||||||||||||||||||
| Project |
n8n
|
| Branch Review |
pay-1727-spike-partial-execution-logic
|
| Run status |
|
| Run duration | 04m 37s |
| Commit |
|
| Committer | Danny Martini |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
1
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
430
|
| View all changes introduced in this branch ↗︎ | |
Co-authored-by: Tomi Turtiainen <10324676+tomi@users.noreply.github.com>
tomi
approved these changes
Sep 18, 2024
|
✅ All Cypress E2E specs passed |
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.
Summary
This PR is rather big.
It does contain TODOs and functions that miss implementation. I'm aware of that, but still find that getting this into trunk now and creating smaller PRs for the TODOs and missing functionality will make future reviewing easier.
It's mostly new files that connect with the old code via one feature gate in the
POST /workflows/runcontroller/service.The new functionality should not have an effect on the old code.
The new code follows the spec written here: https://www.notion.so/n8n/Partial-Executions-9f24ffe8c6474eaeab51c9784ad1fd46?p=4298d34eb54f42e1baa098c9ccc50b5a&pm=s
Changes to the old code are kept to a minimum to avoid breaking the old partial execution flow. The goal is to remove all old code after the new partial executions flow saw enough testing.
I also added some comments to parts of the code that I did not immediately understand.
Important Review Notes
It's best to start reviewing with this file:
packages/core/src/WorkflowExecute.tsEverything in
cypress/*is only done to make the interceptors still work withPOST /workflow/runhaving a query parameter now.All the new code including the tests are in
packages/core/src/PartialExecutionUtilsneatly separated by modules that align with the aforementioned spec.The editor code only contains small adjustments to allow for switching between the old and new flow using a key in local storage.
If you're done with all these, you pretty much reviewed 90% of the PR. Congratulations!
Related Linear tickets, Github issues, and Community forum posts
https://linear.app/n8n/issue/PAY-1755/phase-1-partial-execution-logic-in-basic-workflows
Review / Merge checklist
Docs updated or follow-up ticket created.PR Labeled withrelease/backport(if the PR is an urgent fix that needs to be backported)