Skip to content

Commit 3e480e6

Browse files
committed
suggestion fix
1 parent ae8ebe9 commit 3e480e6

File tree

3 files changed

+54
-20
lines changed

3 files changed

+54
-20
lines changed

cortex/dashboard.py

Lines changed: 51 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,9 @@
1919
from rich.console import Console, Group
2020
from rich.table import Table
2121
from rich.panel import Panel
22-
from rich.layout import Layout
2322
from rich.live import Live
2423
from rich.text import Text
2524
from rich.columns import Columns
26-
from rich.progress import Progress, BarColumn, TextColumn, DownloadColumn
2725
from rich.box import ROUNDED
2826
except ImportError as e:
2927
raise ImportError(f"rich library required: {e}. Install with: pip install rich")
@@ -37,15 +35,20 @@
3735
import pynvml
3836
GPU_AVAILABLE = True
3937
except ImportError:
38+
pynvml = None # type: ignore
4039
GPU_AVAILABLE = False
4140

4241
# Cross-platform keyboard input
4342
if sys.platform == 'win32':
4443
import msvcrt
44+
termios = None # type: ignore
45+
tty = None # type: ignore
46+
select = None # type: ignore
4547
else:
4648
import select
4749
import tty
4850
import termios
51+
msvcrt = None # type: ignore
4952

5053
# Suppress verbose logging
5154
logging.basicConfig(level=logging.WARNING)
@@ -86,7 +89,7 @@ class SystemMetrics:
8689
ram_total_gb: float
8790
gpu_percent: Optional[float] = None
8891
gpu_memory_percent: Optional[float] = None
89-
timestamp: datetime = None
92+
timestamp: Optional[datetime] = None
9093

9194
def __post_init__(self):
9295
if self.timestamp is None:
@@ -115,7 +118,23 @@ def update_elapsed(self):
115118

116119

117120
class SystemMonitor:
118-
"""Monitors CPU, RAM, GPU metrics"""
121+
"""
122+
Monitors CPU, RAM, and GPU metrics in a thread-safe manner.
123+
124+
This class collects system metrics using psutil and, if available, pynvml for GPU monitoring.
125+
Metrics are updated synchronously via `update_metrics()` and accessed via `get_metrics()`.
126+
Thread safety is ensured using a threading.Lock to protect access to the current metrics.
127+
128+
Threading Model:
129+
- All access to metrics is protected by a lock.
130+
- Safe to call `update_metrics()` and `get_metrics()` from multiple threads.
131+
132+
Example:
133+
monitor = SystemMonitor()
134+
monitor.update_metrics()
135+
metrics = monitor.get_metrics()
136+
print(f"CPU: {metrics.cpu_percent}%")
137+
"""
119138

120139
def __init__(self):
121140
self.current_metrics = SystemMetrics(
@@ -130,7 +149,7 @@ def __init__(self):
130149

131150
def _init_gpu(self):
132151
"""Initialize GPU monitoring if available"""
133-
if not GPU_AVAILABLE:
152+
if not GPU_AVAILABLE or pynvml is None:
134153
return
135154
try:
136155
pynvml.nvmlInit()
@@ -152,14 +171,14 @@ def update_metrics(self):
152171
gpu_percent = None
153172
gpu_memory_percent = None
154173

155-
if self.gpu_initialized:
174+
if self.gpu_initialized and pynvml is not None:
156175
try:
157176
device_count = pynvml.nvmlDeviceGetCount()
158177
if device_count > 0:
159178
handle = pynvml.nvmlDeviceGetHandleByIndex(0)
160-
gpu_percent = pynvml.nvmlDeviceGetUtilizationRates(handle).gpu
179+
gpu_percent = float(pynvml.nvmlDeviceGetUtilizationRates(handle).gpu)
161180
mem_info = pynvml.nvmlDeviceGetMemoryInfo(handle)
162-
gpu_memory_percent = (mem_info.used / mem_info.total) * 100
181+
gpu_memory_percent = float(mem_info.used) / float(mem_info.total) * 100
163182
except Exception as e:
164183
logger.debug(f"GPU metrics error: {e}")
165184

@@ -245,7 +264,7 @@ def _load_shell_history(self):
245264
self.history.append(cmd)
246265
break
247266
except Exception as e:
248-
logger.debug(f"History load error: {e}")
267+
logger.warning(f"Could not read history file {history_file}: {e}")
249268

250269
def add_command(self, command: str):
251270
"""Add command to history"""
@@ -261,6 +280,8 @@ def get_history(self) -> List[str]:
261280

262281
class UIRenderer:
263282
"""Renders the dashboard UI with multi-tab support"""
283+
284+
MAX_LIBRARIES_DISPLAY = 5 # Maximum number of libraries to display in progress
264285

265286
def __init__(self, monitor: SystemMonitor, lister: ProcessLister, history: CommandHistory):
266287
self.console = Console()
@@ -437,9 +458,9 @@ def _render_progress_panel(self) -> Panel:
437458

438459
# Show installed libraries for install operations
439460
if progress.libraries and progress.package not in ["System Benchmark", "System Doctor"]:
440-
lines.append(f"\n[dim]Libraries: {', '.join(progress.libraries[:5])}[/dim]")
441-
if len(progress.libraries) > 5:
442-
lines.append(f"[dim]... and {len(progress.libraries) - 5} more[/dim]")
461+
lines.append(f"\n[dim]Libraries: {', '.join(progress.libraries[:self.MAX_LIBRARIES_DISPLAY])}[/dim]")
462+
if len(progress.libraries) > self.MAX_LIBRARIES_DISPLAY:
463+
lines.append(f"[dim]... and {len(progress.libraries) - self.MAX_LIBRARIES_DISPLAY} more[/dim]")
443464

444465
# Status messages
445466
if progress.error_message:
@@ -664,7 +685,6 @@ def _submit_installation_input(self):
664685
package = self.input_text.strip()
665686
self.installation_progress.package = package
666687
self.installation_progress.state = InstallationState.PROCESSING
667-
self.installation_progress.input_active = False
668688
self.input_active = False
669689

670690
# Simulate processing - in real implementation, this would call CLI
@@ -742,8 +762,9 @@ def run(self):
742762
try:
743763
old_settings = termios.tcgetattr(sys.stdin)
744764
tty.setcbreak(sys.stdin.fileno())
745-
except Exception:
746-
pass
765+
except Exception as e:
766+
# It's safe to ignore errors when setting terminal attributes (e.g., not a tty)
767+
logger.debug(f"Failed to set terminal attributes: {e}")
747768

748769
def monitor_loop():
749770
while self.running:
@@ -780,15 +801,25 @@ def monitor_loop():
780801
finally:
781802
self.running = False
782803
# Restore terminal settings on Unix
783-
if old_settings is not None:
804+
if old_settings is not None and termios is not None:
784805
try:
785806
termios.tcsetattr(sys.stdin, termios.TCSADRAIN, old_settings)
786-
except Exception:
787-
pass
807+
except Exception as e:
808+
logger.warning(f"Failed to restore terminal settings: {e}")
788809

789810

790811
class DashboardApp:
791-
"""Main dashboard application"""
812+
"""
813+
Main dashboard application orchestrator.
814+
815+
Coordinates all dashboard components including system monitoring,
816+
process listing, command history, and UI rendering. Provides the
817+
main entry point for running the dashboard.
818+
819+
Example:
820+
app = DashboardApp()
821+
app.run()
822+
"""
792823

793824
def __init__(self):
794825
self.monitor = SystemMonitor()
@@ -805,7 +836,7 @@ def run(self):
805836
time.sleep(1)
806837
self.ui.run()
807838
except KeyboardInterrupt:
808-
pass
839+
console.print("\n[yellow]Keyboard interrupt received. Shutting down dashboard...[/yellow]")
809840
except Exception as e:
810841
console.print(f"[red]Error: {e}[/red]")
811842
finally:

requirements.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ rich>=13.0.0
99

1010
# Type hints for older Python versions
1111
typing-extensions>=4.0.0
12+
PyYaml>=6.0.0
1213

1314
# System monitoring (for dashboard)
1415
psutil>=5.0.0

test/test_dashboard.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ def load_dashboard():
1010
"""Load dashboard module"""
1111
path = os.path.join(os.path.dirname(os.path.dirname(__file__)), "cortex", "dashboard.py")
1212
spec = importlib.util.spec_from_file_location("dashboard", path)
13+
if spec is None or spec.loader is None:
14+
raise ImportError("Failed to load dashboard module")
1315
dashboard = importlib.util.module_from_spec(spec)
1416
spec.loader.exec_module(dashboard)
1517
return dashboard

0 commit comments

Comments
 (0)