feat: add dynamo environment checker script #2419
Conversation
keivenchang
requested review from
biswapanda,
hhzhang16,
hutm,
ishandhanani,
julienmancuso,
mohammedabdulwahhab and
nnshah1
as code owners
WalkthroughAdds a new deploy/dynamo_check.py CLI tool that discovers a Dynamo workspace, inspects components and Python packages, probes Cargo build artifacts, optionally sets PYTHONPATH, tests imports, prints summaries, troubleshooting, and usage examples. Exposes DynamoChecker class with run_all, test_imports, show_usage_examples, show_troubleshooting, show_summary, and a main entrypoint. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant CLI as dynamo_check.py (main)
participant DC as DynamoChecker
participant FS as FileSystem
participant ENV as Env Vars
participant PY as Python Import System
participant CG as Cargo/Targets
U->>CLI: Run with flags (--imports/--examples/--try-pythonpath)
CLI->>DC: Initialize DynamoChecker
DC->>FS: Discover workspace (README, components, lib/bindings, Cargo.toml)
DC->>ENV: Read Cargo env, PYTHONPATH
DC->>FS: Locate components (runtime/framework)
DC->>CG: Detect target dirs, build profile, .so artifacts
DC->>PY: Optionally update sys.path (try-pythonpath)
alt --imports
DC->>PY: Attempt imports per discovered modules
PY-->>DC: Success/ImportError details
else --examples
DC->>U: Print usage examples
else default
DC->>PY: Run full checks incl. imports
end
DC->>U: Print summary, environment tree, troubleshooting
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
deploy/dynamo_check.py (4)
407-409: Improve timestamp comparison precision.Using a 1-second tolerance for timestamp comparison might miss genuinely different timestamps that happen to be close. Consider either using exact comparison or documenting why this tolerance is necessary.
Consider making the comparison more precise or configurable:
- if ( - abs(debug_mtime - release_mtime) < 1.0 - ): # Same timestamp (within 1 second) + # Consider timestamps the same if within 0.01 seconds (filesystem precision) + if abs(debug_mtime - release_mtime) < 0.01: return "debug/release" # Both available, runtime choice depends on invocation
450-451: Consider defensive ordering when modifying sys.path.When inserting paths at the beginning of sys.path, the order matters. Currently, paths are inserted in the order they're discovered, which might not be deterministic across different filesystem implementations.
Consider sorting the paths for consistent behavior:
# Update sys.path for current process if paths: + # Sort paths for consistent ordering + paths.sort() # Add paths to sys.path for immediate effect on imports for path in paths: if path not in sys.path: sys.path.insert(0, path) # Insert at beginning for priority
569-575: Consider using a set for file extension checking.Using a list comprehension with
any()for checking file extensions is less efficient than using a set membership test.# Check if this is a generated file we want to show timestamps for - if any( - module_path.endswith(ext) - for ext in [".so", ".pth", ".dll", ".dylib"] - ): + generated_extensions = {".so", ".pth", ".dll", ".dylib"} + if any(module_path.endswith(ext) for ext in generated_extensions): show_timestamp = True
693-703: Simplify package matching logic.The nested conditions for matching .pth files to packages can be simplified using a more direct approach.
- pth_matches_package = False - if package_name == "ai-dynamo-runtime": - # Look for ai_dynamo_runtime.pth or similar - if ( - "ai_dynamo_runtime" in file.lower() - or file.lower().startswith("ai_dynamo_runtime") - ): - pth_matches_package = True - elif package_name == "ai-dynamo": - # Look for _ai_dynamo.pth or ai_dynamo.pth (but not ai_dynamo_runtime.pth) - if ( - "ai_dynamo" in file.lower() - or "_ai_dynamo" in file.lower() - ) and "runtime" not in file.lower(): - pth_matches_package = True + file_lower = file.lower() + pth_matches_package = ( + (package_name == "ai-dynamo-runtime" and "ai_dynamo_runtime" in file_lower) or + (package_name == "ai-dynamo" and "ai_dynamo" in file_lower and "runtime" not in file_lower) + )
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
deploy/dynamo_check.py(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Copyright Checks
deploy/dynamo_check.py
[error] 1-1: Copyright header check failed for deploy/dynamo_check.py. Step: pwsh /workspace/.github/workflows/copyright-check.ps1. Exit code: 255.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Mirror Repository to GitLab
- GitHub Check: Build and Test - dynamo
🔇 Additional comments (4)
deploy/dynamo_check.py (4)
259-263: Verify PDT offset calculation for daylight saving time.The code assumes PDT is always UTC-7, but Pacific Time alternates between PST (UTC-8) and PDT (UTC-7) depending on the date. This hardcoded offset could produce incorrect timestamps during standard time.
Consider using a more robust approach or documenting this limitation:
# Fallback to local time with manual PDT offset approximation - # PDT is UTC-7, so subtract 7 hours from UTC + # Note: This assumes PDT (UTC-7) and doesn't account for PST (UTC-8) during standard time dt_utc = datetime.datetime.utcfromtimestamp(timestamp) dt_pdt = dt_utc - datetime.timedelta(hours=7) return dt_pdt.strftime("%Y-%m-%d %H:%M:%S PDT")Alternatively, provide more accurate handling:
# Fallback to local time with manual Pacific Time offset - # PDT is UTC-7, so subtract 7 hours from UTC dt_utc = datetime.datetime.utcfromtimestamp(timestamp) - dt_pdt = dt_utc - datetime.timedelta(hours=7) - return dt_pdt.strftime("%Y-%m-%d %H:%M:%S PDT") + # Approximate DST: April-October uses PDT (UTC-7), November-March uses PST (UTC-8) + month = dt_utc.month + if 4 <= month <= 10: + dt_pacific = dt_utc - datetime.timedelta(hours=7) + return dt_pacific.strftime("%Y-%m-%d %H:%M:%S PDT") + else: + dt_pacific = dt_utc - datetime.timedelta(hours=8) + return dt_pacific.strftime("%Y-%m-%d %H:%M:%S PST")
56-61: LGTM! Clean optional dependency handling.Good practice using a try-except block for the optional pytz dependency and setting a flag for later conditional use.
72-77: LGTM! Thoughtful warning suppression.Good attention to user experience by suppressing known warnings from the planner module during import testing.
221-234: LGTM! Consistent path normalization.Excellent implementation of home directory replacement for portable output across different environments.
- Implement hierarchical tree display with Dynamo Environment as root - Merge Python package installation status with Runtime/Framework sections - Add .pth file detection with 'Points to' messages for editable installs - Implement proper tree symbols (├─, └─) for visual hierarchy - Fix package matching logic to prevent cross-contamination between packages - Remove redundant 'Package:' prefix and suppress 'Not found' lines when appropriate - Replace user home directory paths with $HOME for cleaner, portable display - Fix workspace detection to always use absolute paths for consistent $HOME replacement - Improve overall readability and organization of environment status reporting
keivenchang
force-pushed
the
keivenchang/feat_add-dynamo-check-script
branch
from
5e1ceae to
b9a48ee
Compare
- Replace pytz external dependency with standard library zoneinfo - Add proper type annotations for all variables and function parameters - Fix Optional parameter types with explicit Dict[str, Any] annotations - Handle potential None values with type narrowing checks - Replace None defaults with empty strings where appropriate - Ensure all function signatures have complete type information - Script now passes mypy type checking without external dependencies
…y; mypy typing cleanup
|
Approved because change and output looks great! Q: is |
Co-authored-by: Keiven Chang <[email protected]> Signed-off-by: Hannah Zhang <[email protected]>
Overview:
A concise checker that prints key environment info, verifies imports, and gives clear suggestions to fix failures.
Details:
Where should the reviewer start?
deploy/dynamo_check.py
Example:
And for other environments:
Summary by CodeRabbit
New Features
Documentation