Update ASR services setup with new dependencies and configuration#198
Update ASR services setup with new dependencies and configuration#198AnkushMalaker merged 1 commit intomainfrom
Conversation
- Consolidated dependencies into a single setup-requirements.txt for Friend-Lite setup scripts. - Updated command paths in wizard.py to reference the new location of setup-requirements.txt. - Added support for PYTORCH_CUDA_VERSION in asr-services to enhance compatibility. - Introduced init.py for interactive configuration of the ASR service. - Removed outdated setup-requirements.txt files from advanced and speaker-recognition services. - Updated docker-compose.yml files to use the new PYTORCH_CUDA_VERSION variable.
|
@coderabbitai review |
WalkthroughConsolidates setup dependencies into a single root setup-requirements.txt while introducing a new interactive ASR services initialization script in Python. Renames CUDA build arguments to PYTORCH_CUDA_VERSION, updates Docker configurations with cu126 default, detects system CUDA versions, and adds reusable CUDA configuration sharing between services via the wizard. Changes
Sequence DiagramsequenceDiagram
actor User
participant wizard.py
participant init.py
participant nvidia-smi
participant .env System
User->>wizard.py: Run setup
wizard.py->>init.py: Invoke ASRServicesSetup.run()
init.py->>init.py: Print header & sections
init.py->>nvidia-smi: Detect system CUDA version
nvidia-smi-->>init.py: Return CUDA version (or default cu126)
init.py->>User: Display CUDA detection result
init.py->>User: Interactive prompt for CUDA selection<br/>(cu121, cu126, cu128)
User-->>init.py: Select CUDA version
init.py->>init.py: Map selection to PyTorch version
init.py->>init.py: Back up existing .env
init.py->>init.py: Copy .env.template → .env
init.py->>init.py: Update PYTORCH_CUDA_VERSION in .env
init.py->>init.py: Set file permissions (600)
init.py->>User: Show configuration summary
init.py->>User: Display next steps & docker-compose commands
init.py-->>wizard.py: Return
wizard.py-->>User: Setup complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
wizard.py (1)
426-428: Remove extraneous f-string prefixes.These strings have no placeholders, so the
fprefix is unnecessary. Per Ruff F541.- console.print(f"[dim] cd backends/advanced && uv run --with-requirements ../../setup-requirements.txt python init.py[/dim]") - console.print(f"[dim] cd extras/speaker-recognition && uv run --with-requirements ../../setup-requirements.txt python init.py[/dim]") - console.print(f"[dim] cd extras/asr-services && uv run --with-requirements ../../setup-requirements.txt python init.py[/dim]") + console.print("[dim] cd backends/advanced && uv run --with-requirements ../../setup-requirements.txt python init.py[/dim]") + console.print("[dim] cd extras/speaker-recognition && uv run --with-requirements ../../setup-requirements.txt python init.py[/dim]") + console.print("[dim] cd extras/asr-services && uv run --with-requirements ../../setup-requirements.txt python init.py[/dim]")extras/asr-services/init.py (3)
92-128: Movereimport to top of file and fix string version comparison.Two issues:
Per coding guidelines, the
import reon line 113 should be at the top of the file.The string comparison
cuda_ver >= "12.8"is fragile. String comparison works lexicographically, so"9.0" >= "12.1"would incorrectly evaluate toTrue. While current CUDA versions happen to work, this could break with future versions.Add at top of file:
import reAnd consider using tuple comparison for robustness:
if match: major, minor = match.groups() - cuda_ver = f"{major}.{minor}" - - # Map to available PyTorch CUDA versions - if cuda_ver >= "12.8": - return "cu128" - elif cuda_ver >= "12.6": - return "cu126" - elif cuda_ver >= "12.1": - return "cu121" + cuda_ver = (int(major), int(minor)) + + # Map to available PyTorch CUDA versions + if cuda_ver >= (12, 8): + return "cu128" + elif cuda_ver >= (12, 6): + return "cu126" + elif cuda_ver >= (12, 1): + return "cu121"
126-128: Avoid silent failure; consider logging or warning the user.Per coding guidelines, errors should not be silently ignored. The bare
passhides potential issues. Consider at minimum logging a debug message or informing the user that CUDA detection failed.- except (subprocess.SubprocessError, FileNotFoundError): - pass + except (subprocess.SubprocessError, FileNotFoundError) as e: + self.console.print(f"[dim]Could not detect CUDA version: {e}. Using default.[/dim]") return "cu126" # Default fallback to cu126
249-251: Improve exception handling: avoid bareExceptionand use proper logging.Per coding guidelines, always raise errors explicitly rather than silently failing. Per retrieved learnings, prefer
logging.exception()inside except blocks to capture the full stack trace, and chain exceptions withraise ... from ewhen re-raising.Since this is a top-level handler that exits, consider logging the full traceback for debugging:
+import logging + +logger = logging.getLogger(__name__)Then update the exception handler:
except Exception as e: - self.console.print(f"[red][ERROR][/red] Setup failed: {e}") + logger.exception("Setup failed") + self.console.print(f"[red][ERROR][/red] Setup failed: {e}") sys.exit(1)Alternatively, if you want to keep it simple without adding logging infrastructure, at least use
import tracebackand print the traceback for debugging:except Exception as e: + import traceback + traceback.print_exc() self.console.print(f"[red][ERROR][/red] Setup failed: {e}") sys.exit(1)Based on learnings, this improves debuggability.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
extras/asr-services/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
backends/advanced/setup-requirements.txt(0 hunks)extras/asr-services/.env.template(1 hunks)extras/asr-services/Dockerfile_Parakeet(2 hunks)extras/asr-services/docker-compose.yml(1 hunks)extras/asr-services/init.py(1 hunks)extras/asr-services/pyproject.toml(1 hunks)extras/speaker-recognition/docker-compose.yml(1 hunks)extras/speaker-recognition/setup-requirements.txt(0 hunks)setup-requirements.txt(1 hunks)wizard.py(4 hunks)
💤 Files with no reviewable changes (2)
- backends/advanced/setup-requirements.txt
- extras/speaker-recognition/setup-requirements.txt
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Use Black formatter with 100-character line length for Python code
Use isort for import sorting in Python code
ALL imports must be at the top of the file after the docstring. NEVER import modules in the middle of functions or files. Use lazy imports sparingly and only when absolutely necessary for circular import issues.
Group imports in order: standard library, third-party, local imports
Always raise errors, never silently ignore. Use explicit error handling with proper exceptions rather than silent failures.
Do not add defensivehasattr()checks. Research and understand input/response or class structure instead.
Useuv run pythonoruv run python3instead of directpythonorpython3commands for Python execution
Files:
extras/asr-services/init.pywizard.py
**/docker-compose*.{yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Check if src/ is volume mounted. If not, run
docker compose buildso that code changes are reflected. Do not simply rundocker compose restartas it will not rebuild the image.
Files:
extras/asr-services/docker-compose.ymlextras/speaker-recognition/docker-compose.yml
🧠 Learnings (1)
📚 Learning: 2025-12-08T23:52:34.959Z
Learnt from: AnkushMalaker
Repo: chronicler-ai/chronicle PR: 178
File: backends/advanced/src/advanced_omi_backend/services/memory/providers/mycelia.py:218-223
Timestamp: 2025-12-08T23:52:34.959Z
Learning: In Python code (chronicle project), prefer logging.exception() inside except blocks to automatically log the full stack trace. When re-raising exceptions, always chain with 'raise ... from e' to preserve the original context; use 'raise ... from None' only if you explicitly want to suppress the context. This improves debuggability across Python files.
Applied to files:
extras/asr-services/init.pywizard.py
🪛 Ruff (0.14.8)
extras/asr-services/init.py
96-96: Starting a process with a partial executable path
(S607)
104-104: Starting a process with a partial executable path
(S607)
249-249: Do not catch blind exception: Exception
(BLE001)
wizard.py
426-426: f-string without any placeholders
Remove extraneous f prefix
(F541)
427-427: f-string without any placeholders
Remove extraneous f prefix
(F541)
428-428: f-string without any placeholders
Remove extraneous f prefix
(F541)
⏰ 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). (1)
- GitHub Check: speaker-recognition-tests
🔇 Additional comments (13)
extras/speaker-recognition/docker-compose.yml (1)
49-49: No issues identified. PyTorch cu126 is officially supported and available as a valid build configuration. Thesrc/directory is properly volume-mounted, ensuring code changes are reflected without requiring a rebuild.extras/asr-services/pyproject.toml (1)
108-111: LGTM!The explicit
packages = []declaration correctly indicates this project contains only standalone scripts with no installable packages. This prevents setuptools from auto-discovering packages and is a good practice for script-only projects.extras/asr-services/.env.template (1)
4-7: LGTM!Clear documentation for the PYTORCH_CUDA_VERSION variable with appropriate options and guidance for users to verify their system's CUDA version.
setup-requirements.txt (1)
1-5: LGTM!Good consolidation of setup dependencies with appropriate version constraints. The centralized requirements file simplifies dependency management across multiple service init scripts.
extras/asr-services/Dockerfile_Parakeet (1)
6-7: LGTM!The rename from
CUDA_VERSIONtoPYTORCH_CUDA_VERSIONis more descriptive and aligns with the docker-compose and .env.template. The build argument is correctly used in theuv synccommand.Also applies to: 19-20
extras/asr-services/docker-compose.yml (2)
7-7: LGTM!The rename to
PYTORCH_CUDA_VERSIONwith defaultcu126is consistent with related configuration files.
11-15: Note: Code changes require rebuild.As per the coding guidelines, since
src/is not volume-mounted, code changes require runningdocker compose buildto be reflected. The current./model_cache,./debug, and./resultsmounts are appropriate for persistent data, but application code is baked into the image at build time.wizard.py (3)
58-71: LGTM!The command paths are correctly updated to reference the centralized
../../setup-requirements.txt, which aligns with the consolidation of dependencies for all service init scripts.
89-89: LGTM!Adding
asr-servicesto the Python-init-service check ensures proper validation ofinit.pyexistence for the new ASR services setup.
218-224: LGTM!Good UX improvement to reuse
PYTORCH_CUDA_VERSIONfrom an existing speaker-recognition configuration, avoiding redundant user prompts when both services are configured together.extras/asr-services/init.py (3)
1-22: LGTM!Imports are correctly organized at the top of the file with standard library, then third-party packages.
203-222: LGTM!The summary and next steps provide clear, actionable guidance for users after configuration.
254-264: LGTM!Clean CLI entry point with appropriate argument parsing for the
--pytorch-cuda-versionoption.
2 participants
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.