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

refactor(core): Tear down internal hooks (no-changelog) #10340

Merged
merged 4 commits into from
Aug 12, 2024

Conversation

ivov
Copy link
Contributor

@ivov ivov commented Aug 9, 2024

This PR concludes the long effort to decouple event bus and telemetry from internal hooks.

  • No more services pulling multiple unneeded dependencies
  • No more blocking on telemetry and log streaming
  • No more dependency cycles going though internal hooks
  • No more computing unsent telemetry when disabled

Final follow-up to: #10326

@ivov ivov marked this pull request as draft August 9, 2024 09:22
@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Aug 9, 2024
@ivov ivov marked this pull request as ready for review August 9, 2024 10:30
Comment on lines +6 to +7
* @deprecated Do not add to this class. It will be removed once we remove
* further dep cycles. To add log streaming or telemetry events, use
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We cannot yet fully remove this because of this issue, but effectively this is no longer used.

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.

Really nice to see this move over the finish line. Great job in pushing it! 👏 🏅

Copy link
Contributor

✅ All Cypress E2E specs passed

Copy link

cypress bot commented Aug 12, 2024



Test summary

395 0 0 0Flakiness 1


Run details

Project n8n
Status Passed
Commit 3551f74
Started Aug 12, 2024 7:53 AM
Ended Aug 12, 2024 7:58 AM
Duration 04:38 💡
OS Linux Debian -
Browser Electron 118

View run in Cypress Cloud ➡️


Flakiness

e2e/17-sharing.cy.ts Flakiness
1 Credential Usage in Cross Shared Workflows > should only show credentials in their personal project for members if the workflow was shared with them

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud

@ivov ivov merged commit 6b52beb into master Aug 12, 2024
27 checks passed
@ivov ivov deleted the tear-down-internal-hooks branch August 12, 2024 08:13
@janober
Copy link
Member

janober commented Aug 15, 2024

Got released with [email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team Released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants