Repository navigation
fix(multimodal): lazy load package exports - #11186
glamr-agent wants to merge 3 commits into
Conversation
Signed-off-by: Coding Agent <svc-glamr@nvidia.com>
This comment has been minimized.
This comment has been minimized.
WalkthroughThe multimodal package ChangesLazy multimodal imports
Estimated code review effort: 3 (Moderate) | ~20 minutes Related issues: Resolves the eager Suggested labels: enhancement, performance, tests Suggested reviewers: maintainers familiar with Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/src/dynamo/common/tests/multimodal/test_image_loader.py`:
- Around line 43-67: The subprocess-based import test in
test_package_image_loader_import_does_not_import_torch can hang indefinitely
because the subprocess.run call has no timeout. Update this test to guard the
child process with a timeout on subprocess.run, and add the appropriate pytest
timeout marker for the test so it cannot stall in CI. Use the existing test
function name and its subprocess.run invocation as the place to apply the fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4658a066-ff8d-458f-91df-d6883553c4ac
📒 Files selected for processing (2)
components/src/dynamo/common/multimodal/__init__.pycomponents/src/dynamo/common/tests/multimodal/test_image_loader.py
Signed-off-by: Dynamo Agent <dynamo-agent@users.noreply.github.com>
Signed-off-by: Coding Agent <svc-glamr@nvidia.com>
Fixes #11172
Summary
dynamo.common.multimodalre-exports through module-level__getattr__soImageLoaderdoes not eagerly import embedding cache/transfer modules.torchimports while importingImageLoaderfrom the package.Validation
PYTHONPATH=components/src python - <<'PY' ... import dynamo.common.multimodal ... PYpython -m compileall -q components/src/dynamo/common/multimodal/__init__.py components/src/dynamo/common/tests/multimodal/test_image_loader.pygit diff --checkNotes
python -m pytest components/src/dynamo/common/tests/multimodal/test_image_loader.pycould not run in this runner becausepytest,pip, andensurepipare not installed.Summary by CodeRabbit
Bug Fixes
Tests