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

refactor(core): Make placeholders in manual executions in workers temporary #12463

Merged
merged 15 commits into from
Jan 14, 2025
Merged

refactor(core): Make placeholders in manual executions in workers temporary #12463

merged 15 commits into from
Jan 14, 2025

Conversation

ivov
Copy link
Contributor

@ivov ivov commented Jan 6, 2025 •
edited
Loading

Summary

Please see details in story.

To test:

  1. Get workflow and payload attached in Slack.
  2. Set up a dummy server:
mkdir ~/Downloads/dummy
mv ~/Downloads/large-wf-data.json ~/Downloads/dummy
cd ~/Downloads/dummy
python3 -m http.server 3000
  1. Start a main in queue mode with OFFLOAD_MANUAL_EXECUTIONS_TO_WORKERS=true and import the workflow.
  2. Start a worker.
  3. Run the workflow manually.

Expected: See acceptance criteria in the story.

Related Linear tickets, Github issues, and Community forum posts

https://linear.app/n8n/issue/PAY-2429

Review / Merge checklist

  • PR title and summary are descriptive. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.
  • PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

@codecov Codecov
Copy link

codecov bot commented Jan 6, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 20.83333% with 57 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ges/editor-ui/src/composables/usePushConnection.ts 7.14% 39 Missing ⚠️
packages/editor-ui/src/components/RunData.vue 18.18% 9 Missing ⚠️
packages/cli/src/push/index.ts 0.00% 5 Missing ⚠️
packages/editor-ui/src/utils/executionUtils.ts 42.85% 4 Missing ⚠️

📢 Thoughts on this report? Let us know!

@n8n-assistant n8n-assistant bot added n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system labels Jan 6, 2025
@ivov ivov marked this pull request as ready for review January 7, 2025 12:21
tomi
tomi reviewed Jan 14, 2025
View reviewed changes
Copy link
Contributor

@tomi tomi left a comment

Choose a reason for hiding this comment

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

Seems to working nicely 👌 Couple comments / questions

packages/editor-ui/src/composables/usePushConnection.ts Show resolved Hide resolved
packages/editor-ui/src/stores/ui.store.ts
Comment on lines +333 to +337
/**
* Whether we are currently in the process of fetching and deserializing
* the full execution data and loading it to the store.
*/
const isProcessingExecutionResults = computed(() => processingExecutionResults.value);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the correct place for this? Should this rather be in the executions store or pushConnectionStore?

Copy link
Contributor Author

@ivov ivov Jan 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Placed this here as it is similar to isActionActive for showing UI based on what is currently in progress.

@alexgrozav Do you have a preference where this should be?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can do this as a followup if needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks :)

packages/editor-ui/src/stores/workflows.store.ts Outdated Show resolved Hide resolved
packages/editor-ui/src/composables/usePushConnection.ts Outdated Show resolved Hide resolved
@ivov ivov merged commit c2569a0 into master Jan 14, 2025
70 of 74 checks passed
@ivov ivov deleted the pay-2429 branch January 14, 2025 13:32
@janober
Copy link
Member

janober commented Jan 15, 2025

Got released with [email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomi tomi tomi approved these changes

Assignees
No one assigned
Labels
n8n team Authored by the n8n team Released ui Enhancement in /editor-ui or /design-system
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ivov @janober @tomi