fix(infra): create pre commit script and port vertex as lazy import#5453
Merged
edwin-onyx merged 22 commits intomainfrom Sep 22, 2025
Merged
fix(infra): create pre commit script and port vertex as lazy import#5453edwin-onyx merged 22 commits intomainfrom
edwin-onyx merged 22 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
13adb05 to
7c94ce3
Compare
Moving this script to a separate feature branch for better organization. The script will be available in the feature/celery-memory-measurement branch. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
7 issues found across 12 files
Prompt for AI agents (all 7 issues)
Understand the root cause of the following 7 issues and fix them.
<file name="backend/onyx/lazy_handling/lazy_module.py">
<violation number="1" location="backend/onyx/lazy_handling/lazy_module.py:29">
Possible race: getattr may run after a prior import failure by another thread, leading to AttributeError from None instead of ImportError; re-check _import_failed (or ensure self._module is not None) after the import section before getattr.</violation>
</file>
<file name="backend/scripts/check_lazy_imports.py">
<violation number="1" location="backend/scripts/check_lazy_imports.py:110">
Dotted submodule imports like `from google import auth` (for protected `google.auth`) are not detected, allowing disallowed direct imports to slip through.</violation>
</file>
<file name="backend/tests/unit/scripts/test_check_lazy_imports.py">
<violation number="1" location="backend/tests/unit/scripts/test_check_lazy_imports.py:134">
Test enforces over-broad violation: flagging 'from some_package import vertexai' will cause false positives; restrict to imports from the protected module/package only.</violation>
</file>
<file name=".pre-commit-config.yaml">
<violation number="1" location=".pre-commit-config.yaml:44">
Use python3 to ensure the hook runs with Python 3, avoiding failures on systems where "python" is Python 2 or not available.</violation>
</file>
<file name="backend/tests/unit/onyx/lazy_handling/test_lazy_module.py">
<violation number="1" location="backend/tests/unit/onyx/lazy_handling/test_lazy_module.py:68">
Ineffective assertion: comparing the same object to itself is always true, so this test does not verify caching behavior.</violation>
</file>
<file name="backend/onyx/natural_language_processing/search_nlp_models.py">
<violation number="1" location="backend/onyx/natural_language_processing/search_nlp_models.py:276">
Accessing TextEmbeddingModel from top-level lazy_vertexai likely breaks: class lives in vertexai.language_models, but the lazy wrapper only imports 'vertexai' top-level, causing AttributeError at runtime.</violation>
<violation number="2" location="backend/onyx/natural_language_processing/search_nlp_models.py:279">
Accessing TextEmbeddingInput from top-level lazy_vertexai likely fails; it lives in vertexai.language_models, but only the top-level package is lazily imported.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
Weves
reviewed
Sep 19, 2025
…into edwin/dan-2558
Weves
approved these changes
Sep 20, 2025
razvanMiu
pushed a commit
to eea/danswer
that referenced
this pull request
Oct 16, 2025
…nyx-dot-app#5453) Co-authored-by: Claude <noreply@anthropic.com>
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.
Description
created pre commit script to prevent brittleness of inline lazy imports it make sure that packages which are lazy loaded must not be imported directly
couldnt find any solid lazy loading python libraries, top results that come up r smth like this https://pypi.org/project/lazy-imports/ which has liek 10 starts, i think just doing python native and having script to enforce is prob best?
ported over the vertex ai import to this structure and tested using script in https://github.com/onyx-dot-app/onyx/pull/5456/files
per celery worker ->
without vertex ai lazy loading-Peak RSS Memory: 796.9 MB
with - Peak RSS Memory: 653.0 MB, 18% decrease
moving forward will start porting other big packages / connector specific pkgs that likely won't be used by core application flow onto new lazy loading abstraction to further reduce mem usage
https://linear.app/danswer/issue/DAN-2558/investigate-lazy-loading
How Has This Been Tested?
created unit tests for both lazy loading abstraction and pre commit script
Backporting (check the box to trigger backport action)
Note: You have to check that the action passes, otherwise resolve the conflicts manually and tag the patches.
Summary by cubic
Introduces lazy imports to reduce baseline memory and only load heavy Google/VertexAI libs when needed, addressing DAN-2558 (Reduce Memory usage in Onyx). Adds tools to measure Celery worker memory and per-package import cost.
Refactors
New Features