-
Notifications
You must be signed in to change notification settings - Fork 25
Fix/wizard nits #134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix/wizard nits #134
Changes from all commits
aff7e16
fb1be97
93b7572
afac121
d65aad6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,7 @@ | |
| from pathlib import Path | ||
| from typing import Any, Dict | ||
|
|
||
| from dotenv import set_key | ||
| from dotenv import get_key, set_key | ||
| from rich.console import Console | ||
| from rich.panel import Panel | ||
| from rich.prompt import Confirm, Prompt | ||
|
|
@@ -100,6 +100,26 @@ def backup_existing_env(self): | |
| shutil.copy2(env_path, backup_path) | ||
| self.console.print(f"[blue][INFO][/blue] Backed up existing .env file to {backup_path}") | ||
|
|
||
| def read_existing_env_value(self, key: str) -> str: | ||
| """Read a value from existing .env file""" | ||
| env_path = Path(".env") | ||
| if not env_path.exists(): | ||
| return None | ||
|
|
||
| value = get_key(str(env_path), key) | ||
| # get_key returns None if key doesn't exist or value is empty | ||
| return value if value else None | ||
|
|
||
| def mask_api_key(self, key: str, show_chars: int = 5) -> str: | ||
| """Mask API key showing only first and last few characters""" | ||
| if not key or len(key) <= show_chars * 2: | ||
| return key | ||
|
|
||
| # Remove quotes if present | ||
| key_clean = key.strip("'\"") | ||
|
|
||
| return f"{key_clean[:show_chars]}{'*' * min(15, len(key_clean) - show_chars * 2)}{key_clean[-show_chars:]}" | ||
|
|
||
| def setup_authentication(self): | ||
| """Configure authentication settings""" | ||
| self.print_section("Authentication Setup") | ||
|
|
@@ -128,8 +148,17 @@ def setup_transcription(self): | |
| if choice == "1": | ||
| self.console.print("[blue][INFO][/blue] Deepgram selected") | ||
| self.console.print("Get your API key from: https://console.deepgram.com/") | ||
|
|
||
| api_key = self.prompt_value("Deepgram API key (leave empty to skip)", "") | ||
|
|
||
| # Check for existing API key | ||
| existing_key = self.read_existing_env_value("DEEPGRAM_API_KEY") | ||
| if existing_key and existing_key not in ['your_deepgram_api_key_here', 'your-deepgram-key-here']: | ||
| masked_key = self.mask_api_key(existing_key) | ||
| prompt_text = f"Deepgram API key ({masked_key}) [press Enter to reuse, or enter new]" | ||
| api_key_input = self.prompt_value(prompt_text, "") | ||
| api_key = api_key_input if api_key_input else existing_key | ||
| else: | ||
| api_key = self.prompt_value("Deepgram API key (leave empty to skip)", "") | ||
|
|
||
|
Comment on lines
+152
to
+161
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Collect new API keys via hidden input, not echoed prompts. Deepgram/Mistral/OpenAI new-key paths use Prompt.ask, which displays secrets on screen and persists in terminal history. Use getpass-based prompt_password for new entries; keep masked reuse flow. - api_key = self.prompt_value("Deepgram API key (leave empty to skip)", "")
+ api_key = self.prompt_password("Deepgram API key (leave empty to skip)") or ""
@@
- api_key = self.prompt_value("Mistral API key (leave empty to skip)", "")
+ api_key = self.prompt_password("Mistral API key (leave empty to skip)") or ""
@@
- api_key = self.prompt_value("OpenAI API key (leave empty to skip)", "")
+ api_key = self.prompt_password("OpenAI API key (leave empty to skip)") or ""Also applies to: 174-183, 220-229 🤖 Prompt for AI Agents |
||
| if api_key: | ||
| self.config["TRANSCRIPTION_PROVIDER"] = "deepgram" | ||
| self.config["DEEPGRAM_API_KEY"] = api_key | ||
|
|
@@ -141,10 +170,19 @@ def setup_transcription(self): | |
| self.config["TRANSCRIPTION_PROVIDER"] = "mistral" | ||
| self.console.print("[blue][INFO][/blue] Mistral selected") | ||
| self.console.print("Get your API key from: https://console.mistral.ai/") | ||
|
|
||
| api_key = self.prompt_value("Mistral API key (leave empty to skip)", "") | ||
|
|
||
| # Check for existing API key | ||
| existing_key = self.read_existing_env_value("MISTRAL_API_KEY") | ||
| if existing_key and existing_key not in ['your_mistral_api_key_here', 'your-mistral-key-here']: | ||
| masked_key = self.mask_api_key(existing_key) | ||
| prompt_text = f"Mistral API key ({masked_key}) [press Enter to reuse, or enter new]" | ||
| api_key_input = self.prompt_value(prompt_text, "") | ||
| api_key = api_key_input if api_key_input else existing_key | ||
| else: | ||
| api_key = self.prompt_value("Mistral API key (leave empty to skip)", "") | ||
|
|
||
| model = self.prompt_value("Mistral model", "voxtral-mini-2507") | ||
|
|
||
| if api_key: | ||
| self.config["MISTRAL_API_KEY"] = api_key | ||
| self.config["MISTRAL_MODEL"] = model | ||
|
|
@@ -178,11 +216,20 @@ def setup_llm(self): | |
| self.config["LLM_PROVIDER"] = "openai" | ||
| self.console.print("[blue][INFO][/blue] OpenAI selected") | ||
| self.console.print("Get your API key from: https://platform.openai.com/api-keys") | ||
|
|
||
| api_key = self.prompt_value("OpenAI API key (leave empty to skip)", "") | ||
|
|
||
| # Check for existing API key | ||
| existing_key = self.read_existing_env_value("OPENAI_API_KEY") | ||
| if existing_key and existing_key not in ['your_openai_api_key_here', 'your-openai-key-here']: | ||
| masked_key = self.mask_api_key(existing_key) | ||
| prompt_text = f"OpenAI API key ({masked_key}) [press Enter to reuse, or enter new]" | ||
| api_key_input = self.prompt_value(prompt_text, "") | ||
| api_key = api_key_input if api_key_input else existing_key | ||
| else: | ||
| api_key = self.prompt_value("OpenAI API key (leave empty to skip)", "") | ||
|
|
||
| model = self.prompt_value("OpenAI model", "gpt-4o-mini") | ||
| base_url = self.prompt_value("OpenAI base URL (for proxies/compatible APIs)", "https://api.openai.com/v1") | ||
|
|
||
| if api_key: | ||
| self.config["OPENAI_API_KEY"] = api_key | ||
| self.config["OPENAI_MODEL"] = model | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,2 @@ | ||
| #!/bin/bash | ||
| uv run --with-requirements requirements.txt python connect-laptop.py | ||
| uv run --with-requirements requirements.txt python connect-omi.py |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,7 @@ services: | |
| context: . | ||
| dockerfile: Dockerfile | ||
| args: | ||
| CUDA_VERSION: ${CUDA_VERSION:-cpu} | ||
| PYTORCH_CUDA_VERSION: ${PYTORCH_CUDA_VERSION:-cpu} | ||
| image: speaker-recognition:latest | ||
| env_file: | ||
| - .env | ||
|
|
@@ -46,7 +46,7 @@ services: | |
| context: . | ||
| dockerfile: Dockerfile | ||
| args: | ||
| CUDA_VERSION: ${CUDA_VERSION:-cu121} | ||
| PYTORCH_CUDA_VERSION: ${PYTORCH_CUDA_VERSION:-cu121} | ||
| deploy: | ||
| resources: | ||
| reservations: | ||
|
|
@@ -108,3 +108,4 @@ services: | |
| networks: | ||
| default: | ||
| name: friend-network | ||
| external: true | ||
|
Comment on lines
109
to
+111
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainexternal: true requires pre-created network; fresh runs may fail. Declaring networks.default as external means friend-network must already exist. Starting this compose in isolation will error. Options:
networks:
default:
name: friend-network
- external: true
+ # Set to true only if the network is managed externally
+ external: ${EXTERNAL_NETWORK:-false}Run to verify failure/success scenarios: 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
# 1) Expect failure when network absent and external=true
docker network rm friend-network >/dev/null 2>&1 || true
echo "Expect failure:"; docker compose -f extras/speaker-recognition/docker-compose.yml config >/dev/null && echo "OK (unexpected)" || echo "Failed as expected"
# 2) Create network and expect success
docker network create friend-network >/dev/null 2>&1 || true
echo "Expect success:"; docker compose -f extras/speaker-recognition/docker-compose.yml config >/dev/null && echo "OK (expected)"Length of output: 140 I need to examine the actual docker-compose.yml file since Docker isn't available in the sandbox to run the verification script. Let me inspect the current state: Length of output: 283 🏁 Script executed: #!/bin/bash
# Get full networks section to understand complete context
grep -n "networks:" extras/speaker-recognition/docker-compose.yml -A 10Length of output: 349 🏁 Script executed: #!/bin/bash
# Check if external: true exists anywhere in the file
grep -n "external:" extras/speaker-recognition/docker-compose.ymlLength of output: 22 Let me check for setup documentation and override files that might handle the network creation: Length of output: 96 🏁 Script executed: #!/bin/bash
# Search for "friend-network" references in the project
rg "friend-network" extras/speaker-recognition/ -nLength of output: 74 🏁 Script executed: #!/bin/bash
# Check for setup/initialization scripts in the speaker-recognition directory
find extras/speaker-recognition -type f \( -name "*.sh" -o -name "Makefile" -o -name "*.md" \) | head -20Length of output: 422 🏁 Script executed: #!/bin/bash
# Look for README or setup documentation
find extras/speaker-recognition -maxdepth 2 -name "README*" -o -name "SETUP*" -o -name "*.md"Length of output: 217 Let me check the alternative compose file and setup scripts to understand the intended network management: Length of output: 0 🏁 Script executed: #!/bin/bash
# Search setup.sh for network creation logic
grep -n "network\|friend-network" extras/speaker-recognition/setup.shLength of output: 0 🏁 Script executed: #!/bin/bash
# Search init.sh for network creation logic
grep -n "network\|friend-network" extras/speaker-recognition/init.shLength of output: 0 🏁 Script executed: #!/bin/bash
# Search README for network setup instructions
grep -n "network\|friend-network\|docker network" extras/speaker-recognition/README.mdLength of output: 271 🏁 Script executed: #!/bin/bash
# Check quickstart.md for network setup instructions
grep -n "network\|friend-network\|docker network" extras/speaker-recognition/quickstart.mdLength of output: 47 Let me verify the actual contents of these setup scripts and the test compose file: Length of output: 621 🏁 Script executed: #!/bin/bash
# View setup.sh to see what it does
head -30 extras/speaker-recognition/setup.shLength of output: 586 🏁 Script executed: #!/bin/bash
# View init.sh to see what it does
head -30 extras/speaker-recognition/init.shLength of output: 802 🏁 Script executed: #!/bin/bash
# Check if there are any docker network create commands anywhere in the project
rg "docker network create" extras/speaker-recognition/Length of output: 0 🏁 Script executed: #!/bin/bash
# Check run-test.sh for network setup
grep -n "network\|friend-network" extras/speaker-recognition/run-test.shLength of output: 0 Network marked external but no creation mechanism or documentation. The concern is valid.
Fresh deployments will fail. The proposed env var solution is sound—it allows external management when needed while defaulting to auto-creation. networks:
default:
name: friend-network
- external: true
+ external: ${EXTERNAL_NETWORK:-false}🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,6 +65,27 @@ def prompt_password(self, prompt: str) -> str: | |
| self.console.print("[red][ERROR][/red] Token is required for speaker recognition") | ||
| sys.exit(1) | ||
|
|
||
| def read_existing_env_value(self, key: str) -> str: | ||
| """Read a value from existing .env file""" | ||
| env_path = Path(".env") | ||
| if not env_path.exists(): | ||
| return None | ||
|
|
||
| from dotenv import get_key | ||
| value = get_key(str(env_path), key) | ||
| # get_key returns None if key doesn't exist or value is empty | ||
| return value if value else None | ||
|
|
||
| def mask_api_key(self, key: str, show_chars: int = 5) -> str: | ||
| """Mask API key showing only first and last few characters""" | ||
| if not key or len(key) <= show_chars * 2: | ||
| return key | ||
|
|
||
| # Remove quotes if present | ||
| key_clean = key.strip("'\"") | ||
|
|
||
| return f"{key_clean[:show_chars]}{'*' * min(15, len(key_clean) - show_chars * 2)}{key_clean[-show_chars:]}" | ||
|
|
||
| def prompt_choice(self, prompt: str, choices: Dict[str, str], default: str = "1") -> str: | ||
| """Prompt for a choice from options""" | ||
| self.console.print(prompt) | ||
|
|
@@ -103,9 +124,65 @@ def setup_hf_token(self): | |
| self.config["HF_TOKEN"] = self.args.hf_token | ||
| self.console.print("[green][SUCCESS][/green] HF Token configured from command line") | ||
| else: | ||
| hf_token = self.prompt_password("HF Token") | ||
| self.config["HF_TOKEN"] = hf_token | ||
| self.console.print("[green][SUCCESS][/green] HF Token configured") | ||
| # Check for existing token | ||
| existing_token = self.read_existing_env_value("HF_TOKEN") | ||
| if existing_token and existing_token not in ['your_huggingface_token_here', 'your-hf-token-here']: | ||
| masked_token = self.mask_api_key(existing_token) | ||
| self.console.print(f"[blue][INFO][/blue] Found existing token: {masked_token}") | ||
| try: | ||
| reuse = Confirm.ask("Use existing HF Token?", default=True) | ||
| except EOFError: | ||
| reuse = True | ||
|
|
||
| if reuse: | ||
| self.config["HF_TOKEN"] = existing_token | ||
| self.console.print("[green][SUCCESS][/green] HF Token configured (reused)") | ||
| else: | ||
| hf_token = self.prompt_password("HF Token") | ||
| self.config["HF_TOKEN"] = hf_token | ||
| self.console.print("[green][SUCCESS][/green] HF Token configured") | ||
| else: | ||
| hf_token = self.prompt_password("HF Token") | ||
| self.config["HF_TOKEN"] = hf_token | ||
| self.console.print("[green][SUCCESS][/green] HF Token configured") | ||
|
|
||
| def detect_cuda_version(self) -> str: | ||
| """Detect system CUDA version from nvidia-smi""" | ||
| try: | ||
| result = subprocess.run( | ||
| ["nvidia-smi", "--query-gpu=driver_version", "--format=csv,noheader"], | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=5 | ||
| ) | ||
| if result.returncode == 0: | ||
| # Try to get CUDA version from nvidia-smi | ||
| result = subprocess.run( | ||
| ["nvidia-smi"], | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=5 | ||
| ) | ||
| if result.returncode == 0: | ||
| output = result.stdout | ||
| # Parse CUDA Version from nvidia-smi output | ||
| # Format: "CUDA Version: 12.6" | ||
| import re | ||
| match = re.search(r'CUDA Version:\s*(\d+)\.(\d+)', output) | ||
| 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" | ||
| except (subprocess.SubprocessError, FileNotFoundError): | ||
| pass | ||
| return "cu121" # Default fallback | ||
|
|
||
|
Comment on lines
+149
to
186
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CUDA detection compares version strings; use numeric parsing. String comparisons like "12.10" >= "12.8" are wrong lexicographically. Parse major/minor as ints and map reliably; also use shutil.which to skip when nvidia-smi isn’t present (addresses Ruff S607 intent). - def detect_cuda_version(self) -> str:
+ def detect_cuda_version(self) -> str:
"""Detect system CUDA version from nvidia-smi"""
- try:
- result = subprocess.run(
- ["nvidia-smi", "--query-gpu=driver_version", "--format=csv,noheader"],
- capture_output=True,
- text=True,
- timeout=5
- )
- if result.returncode == 0:
- # Try to get CUDA version from nvidia-smi
- result = subprocess.run(
- ["nvidia-smi"],
- capture_output=True,
- text=True,
- timeout=5
- )
- if result.returncode == 0:
- output = result.stdout
- # Parse CUDA Version from nvidia-smi output
- # Format: "CUDA Version: 12.6"
- import re
- match = re.search(r'CUDA Version:\s*(\d+)\.(\d+)', output)
- 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"
- except (subprocess.SubprocessError, FileNotFoundError):
- pass
+ try:
+ import shutil, re
+ if not shutil.which("nvidia-smi"):
+ return "cu121"
+ result = subprocess.run(["nvidia-smi"], capture_output=True, text=True, timeout=5)
+ if result.returncode != 0:
+ return "cu121"
+ match = re.search(r'CUDA Version:\s*(\d+)\.(\d+)', result.stdout)
+ if not match:
+ return "cu121"
+ major, minor = (int(match.group(1)), int(match.group(2)))
+ # Map to available PyTorch CUDA versions
+ if (major, minor) >= (12, 8):
+ return "cu128"
+ if (major, minor) >= (12, 6):
+ return "cu126"
+ if (major, minor) >= (12, 1):
+ return "cu121"
+ except (subprocess.SubprocessError, FileNotFoundError, ValueError):
+ return "cu121"
return "cu121" # Default fallback🧰 Tools🪛 Ruff (0.14.1)153-153: Starting a process with a partial executable path (S607) 161-161: Starting a process with a partial executable path (S607) |
||
| def setup_compute_mode(self): | ||
| """Configure compute mode (CPU/GPU)""" | ||
|
|
@@ -125,14 +202,44 @@ def setup_compute_mode(self): | |
|
|
||
| self.config["COMPUTE_MODE"] = compute_mode | ||
|
|
||
| # Set CUDA_VERSION for Docker build | ||
| # Set PYTORCH_CUDA_VERSION for Docker build | ||
| if compute_mode == "cpu": | ||
| self.config["CUDA_VERSION"] = "cpu" | ||
| self.config["PYTORCH_CUDA_VERSION"] = "cpu" | ||
| else: | ||
| # Default to cu121 for GPU mode (can be overridden in .env) | ||
| self.config["CUDA_VERSION"] = "cu121" | ||
| # Detect system CUDA version and suggest as default | ||
| detected_cuda = self.detect_cuda_version() | ||
|
|
||
| # Map to default choice number | ||
| cuda_to_choice = { | ||
| "cu121": "1", | ||
| "cu126": "2", | ||
| "cu128": "3" | ||
| } | ||
| default_choice = cuda_to_choice.get(detected_cuda, "2") | ||
|
|
||
| self.console.print() | ||
| self.console.print(f"[blue][INFO][/blue] Detected CUDA version: {detected_cuda}") | ||
| self.console.print() | ||
|
|
||
| cuda_choices = { | ||
| "1": "CUDA 12.1 (cu121)", | ||
| "2": "CUDA 12.6 (cu126)", | ||
| "3": "CUDA 12.8 (cu128)" | ||
| } | ||
| cuda_choice = self.prompt_choice( | ||
| "Choose CUDA version for PyTorch:", | ||
| cuda_choices, | ||
| default_choice | ||
| ) | ||
|
|
||
| choice_to_cuda = { | ||
| "1": "cu121", | ||
| "2": "cu126", | ||
| "3": "cu128" | ||
| } | ||
| self.config["PYTORCH_CUDA_VERSION"] = choice_to_cuda[cuda_choice] | ||
|
|
||
| self.console.print(f"[blue][INFO][/blue] Using {compute_mode.upper()} mode") | ||
| self.console.print(f"[blue][INFO][/blue] Using {compute_mode.upper()} mode with PyTorch CUDA version: {self.config['PYTORCH_CUDA_VERSION']}") | ||
|
|
||
| def setup_deepgram(self): | ||
| """Configure Deepgram API key if provided""" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Masking leaks short keys; always mask at least partially.
Current logic returns the raw key if length <= 2*show_chars, which can expose secrets in prompts/logs.
🤖 Prompt for AI Agents