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

Tidy app entrypoint #7668

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Tidy app entrypoint #7668

wants to merge 7 commits into from

Conversation

RyanJDick
Copy link
Collaborator

Summary

Prior to this PR, most of the app setup was being done in api_app.py at import time. This PR cleans this up, by:

  • Splitting app setup into more modular functions
  • Narrower responsibility for the api_app.py file - it just initializes the FastAPI app

The main motivation for this changes is to make it easier to support an upcoming torch configuration feature that requires more careful ordering of app initialization steps.

Related Issues / Discussions

N/A

QA Instructions

  • Launch the app via invokeai-web.py and smoke test it.
  • Launch the app via the installer and smoke test it.
  • Test that generate_openapi_schema.py produces the same result before and after the change.
  • No regression in unit tests that directly interact with the app. (test_images.py)

Merge Plan

  • Check to see if there are any commercial implications to modifying the app entrypoint.

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)

@github-actions github-actions bot added the python PRs that change python files label Feb 21, 2025

import torch

from invokeai.backend.util.logging import InvokeAILogger
Copy link
Contributor

Choose a reason for hiding this comment

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

might be worth inlining this import to the one function that uses it, to prevent this module with the monkeypatch function from loading any more than it strictly needs to.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done in 9ba2713

…esponsible for just initializing the FastAPI app. This also gives clearer control over the order of the initialization steps, which will be important as we add planned torch configurations that must be applied before torch is imported.
@psychedelicious psychedelicious self-requested a review February 27, 2025 01:17
Copy link
Collaborator

@psychedelicious psychedelicious left a comment

Choose a reason for hiding this comment

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

I've improved custom node loading in #7698, moving that code to a function instead of running it implicitly as python loads modules.

Maybe should be re-organised per your other changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python PRs that change python files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants