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

Fix circular imports related to SCHEDULER_NAME_VALUES #6582

Merged
merged 1 commit into from
Jul 5, 2024

Conversation

RyanJDick
Copy link
Collaborator

Summary

This PR makes the following changes:

  • Moves SCHEDULER_NAME_VALUES to invokeai/backend/. Previously, it was located in invokeai/app/invocations/. In its old location, it was causing bidirectional imports between invokeai/backend/ and invokeai/app/invocations/ and thus contributing to circular import issues. This change helps to move towards having invokeai/app/invocations/ import from invokeai/backend/ and not the other way around. (More info on why this is important here: Improve InvokeAI import patterns #6575)
  • Fixes all the static type checking errors caused by doing Literal[tuple(SCHEDULER_MAP.keys())].

Related Issues / Discussions

An issue was raised on Discord where custom nodes fail to import due to a circular import error: https://discord.com/channels/1020123559063990373/1049495067846524939/1258727169417543752

This custom node import issue was introduced by #6576 , which began enforcing absolute imports. The change to absolute imports changed the order of imports in some files, which caused the circular import - highlighting yet again the fragility of the current import system.

Note: This change fixes one source of circular imports. I'm sure that others exist. I'm hopeful that this will fix most custom nodes, but there's a risk that it won't.

QA Instructions

I tested this change with the https://github.com/skunkworxdark/XYGrid_nodes custom nodes. On main, loading these nodes caused an import error. After this PR, I was able to run this workflow with custom nodes successfully: https://github.com/skunkworxdark/XYGrid_nodes/blob/ac72baf3472f47112fbbcb9ef0eff574a0ee267b/workflows/xygrid_model-cfg_wf.json

I also verified that the list of Schedulers still populates properly in the UI (both on nodes, and in the Linear UI).

Merge Plan

No special instructions.

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Tests added / updated (if applicable)
  • Documentation added / updated (if applicable)

…ctional cross-directory imports, which contribute to circular import issues.
@github-actions github-actions bot added python PRs that change python files invocations PRs that change invocations backend PRs that change backend files python-tests PRs that change python tests labels Jul 5, 2024
@hipsterusername hipsterusername enabled auto-merge (rebase) July 5, 2024 14:27
@hipsterusername hipsterusername merged commit 35f8781 into main Jul 5, 2024
14 checks passed
@hipsterusername hipsterusername deleted the ryan/fix-scheduler-circular-import branch July 5, 2024 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend PRs that change backend files invocations PRs that change invocations python PRs that change python files python-tests PRs that change python tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants