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

feat(app): less janky custom node loading #7748

Merged

Conversation

psychedelicious
Copy link
Collaborator

Summary

  • We don't need to copy the init file. Just crawl the custom nodes dir for modules and import them all. Dunno why I didn't do this initially.
  • Pass the logger in as an arg. There was a race condition where if we got the logger directly in the load_custom_nodes function, the config would not have been loaded fully yet and we'd end up with the wrong custom nodes path!
  • Remove permissions-setting logic, I do not believe it is relevant for custom nodes
  • Minor cleanup of the utility

Related Issues / Discussions

n/a

QA Instructions

Your custom nodes should still work like they did before.

Merge Plan

n/a

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Tests added / updated (if applicable)
  • Documentation added / updated (if applicable)
  • Updated What's New copy (if doing a release after this PR)

Copy link
Member

@ebr ebr left a comment

Choose a reason for hiding this comment

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

LGTM, would like someone with a good collection of custom nodes to take this for a spin

@psychedelicious psychedelicious force-pushed the psyche/feat/app/less-janky-custom-node-loading branch from 509d77f to 2448e48 Compare March 7, 2025 06:55
- We don't need to copy the init file. Just crawl the custom nodes dir for modules and import them all. Dunno why I didn't do this initially.
- Pass the logger in as an arg. There was a race condition where if we got the logger directly in the load_custom_nodes function, the config would not have been loaded fully yet and we'd end up with the wrong custom nodes path!
- Remove permissions-setting logic, I do not believe it is relevant for custom nodes
- Minor cleanup of the utility
@psychedelicious psychedelicious force-pushed the psyche/feat/app/less-janky-custom-node-loading branch from 2448e48 to 1939ccd Compare March 7, 2025 22:34
@psychedelicious psychedelicious enabled auto-merge (rebase) March 7, 2025 22:34
@psychedelicious
Copy link
Collaborator Author

Several node enjoyers tried the PR out and it works fine for them

@psychedelicious psychedelicious merged commit 59a8c0d into main Mar 7, 2025
15 checks passed
@psychedelicious psychedelicious deleted the psyche/feat/app/less-janky-custom-node-loading branch March 7, 2025 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invocations PRs that change invocations python PRs that change python files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug]: load_custom_nodes fails and prevents Invoke from starting
3 participants