Improve import startup with lazy top-level exports#2950
Open
fede-kamel wants to merge 2 commits intoopenai:mainfrom
Open
Improve import startup with lazy top-level exports#2950fede-kamel wants to merge 2 commits intoopenai:mainfrom
fede-kamel wants to merge 2 commits intoopenai:mainfrom
Conversation
1 task
Author
|
Validation summary for #2819:
Interpretation: default lazy mode reduces import startup work significantly; eager mode remains available for CI/dev verification. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2bd0b97246
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Two things, about the env var for eager load
I think it's great you're working on a fix for this btw... thank you |
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
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
This PR addresses import-time startup regression tracked in #2819 by making heavy top-level exports lazy.
Key changes:
openai.__init__for:AzureOpenAIAsyncAzureOpenAIpydantic_function_toolAssistantEventHandlerAsyncAssistantEventHandlerOPENAI_EAGER_IMPORT=1scripts/bench_import.py.tests/test_import_surface.pytests/lib/test_import_surface_live.py(live-gated)Why this relates to #2819
Issue #2819 reports that
import openaiis too slow and that eager loading of type-heavy/internal surfaces is a major contributor. This PR reduces startup cost in the default path by moving expensive imports behind first-use access while preserving API compatibility.Testing done
Unit / behavior tests
PYTHONPATH=src python3 -m pytest tests/test_import_surface.py -q -o addopts=''-> passedPYTHONPATH=src python3 -m pytest tests/test_module_client.py -q -o addopts=''-> passedLive API-key integration test
OPENAI_LIVE=1 PYTHONPATH=src OPENAI_API_KEY=... python3 -m pytest tests/lib/test_import_surface_live.py -q -o addopts=''-> passedclient.models.list()).Performance benchmarks
Using
scripts/bench_import.py:0.1533sOPENAI_EAGER_IMPORT=1):0.2601s+0.1068s(+69.7%)0.7019s2.9429s+2.2410s(+319.3%)Interpretation: eager mode is intentionally slower, and the default lazy path materially reduces startup work compared with forced eager loading.
Notes
OPENAI_LIVE=1andOPENAI_API_KEY.