Repository navigation
fix(ci): install networkx in test images - #14771
Conversation
|
👋 Hi bzsuni! Thank you for contributing to ai-dynamo/dynamo. Just a reminder: The 🚀 |
nnshah1
left a comment
There was a problem hiding this comment.
Approving — correct, minimal fix. This resolves the networkx collection error that fails every CPU test job: benchmarks/router/tests/test_common.py transitively imports networkx via benchmarks.router.common → prefix_data_generator.synthesizer, and it wasn't installed in the test image. Verified the whole chain against current main.
This unblocks the repo-wide CI failures alongside #14760 (the framework startup fix).
Nit (non-blocking): most neighbors in this file pin exact versions (nats-py==2.12.0, etc.); consider pinning networkx==X.Y.Z for reproducibility. Fine to land as-is given it's an urgent unblock.
The one red check here is lychee (Docs-link-check), which is a separate known-broken job unrelated to this change.
— Neelay + 🤖
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. WalkthroughThe test-only requirements add the unpinned ChangesRouter benchmark test dependency
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This test-only dependency enables router benchmark collection without changing production runtime dependencies; no concrete merge-blocking risk is evidenced. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Comment |
biswapanda
left a comment
There was a problem hiding this comment.
lgtm. Thanks for fixing this issue.
|
/ok to test fb5b1d9 |
Router benchmark tests import graph synthesis during pytest collection, before marker filtering excludes unrelated tests. Lean CPU test images lack networkx, so this import aborts collection in Triton and Planner jobs. Declare networkx in the shared test requirements installed by Dockerfile.test and development images. This keeps benchmark tests collectable without skipping them and leaves production runtime requirements unchanged. Signed-off-by: bzsuni <bingzhe.sun@daocloud.io>
fb5b1d9 to
844e19e
Compare
|
/ok to test 5b37619 |
Overview:
Add
networkxtocontainer/deps/requirements.test.txt.benchmarks/router/tests/test_common.pyimportsbenchmarks.router.common, which eventually importsnetworkx. Pytest collects the test tree before marker filtering, so whennetworkxis missing, collection fails before the Triton and Planner CPU test selections can run.This is the collection failure that became visible after #14760 fixed the PR workflow startup failure.
Details:
container/Dockerfile.testinstalls the shared test requirements used by these CPU test images. Addingnetworkxthere keeps the router benchmark module collectable without changing production runtime dependencies.#14671 takes a different approach by skipping this benchmark module when
networkxis unavailable. This PR installs the test dependency instead.Validation:
networkx3.6.1.Where should the reviewer start?
container/deps/requirements.test.txtRelated Issues
🚫 This PR is NOT linked to an issue:
Summary by CodeRabbit
networkxpackage as a test dependency for router benchmark coverage.