feat(app): improved custom node loading #7698
Merged
Conversation
This file contains hidden or 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
psychedelicious
requested review from
blessedcoolant,
brandonrising and
hipsterusername
as code owners
github-actions
bot
added
python
hipsterusername
approved these changes
Feb 27, 2025
Previously, custom node loading occurred _during module imports_. A consequence of this is that when a custom node import fails (e.g. its type clobbers an existing node), the app fails to start up. In fact, any time we import basically anything from the app, we trigger custom node imports! Not good. This logic is now in its own function, called as the API app starts up. If a custom node load fails for any reason, it no longer prevents the app from starting up. One other bonus we get from this is that we can now ensure custom nodes are loaded _after_ core nodes. Any clobbering that may occur while loading custom nodes is now guaranteed to be a custom node clobbering a core node's type - and not the other way round.
psychedelicious
force-pushed
the
psyche/feat/app/improved-custom-node-loading
branch
from
6050d35 to
eb00516
Compare
psychedelicious
deleted the
psyche/feat/app/improved-custom-node-loading
branch
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
When we adopt the @skunkworxdark 's metadata nodes in #7697 , we create an unpleasant user experience for those who have the custom node pack installed already. Invoke will fail to start up because there is an error during custom node loading - in this case, because the nodes already exist.
This PR changes how custom node loading works to avoid this. Any error during custom node loading is now logged and we bail out of loading that node pack. Other node packs are still loaded, and the app will work even if a custom node errors.
Also improved messages for node registration errors and custom node loading.
Related Issues / Discussions
n/a
QA Instructions
This test only works once #7697 is merged and this PR is rebased on
main.On
main, we expect Invoke to not start up at all.On this PR, we expect an error to be logged and Invoke to start up just fine.
Merge Plan
n/a
Checklist
What's Newcopy (if doing a release after this PR)