Skip to content

Conversation

@subCode321
Copy link
Contributor

@subCode321 subCode321 commented Mar 12, 2025

Fixes: #124

  • Used st.fragment with re-run to check cpu and ram stats every 5 seconds
  • Added this new section to the sidebar

Recording

Screen.Recording.2025-03-12.at.9.04.39.PM.mov

Summary by CodeRabbit

  • New Features
    • Added a new sidebar section that displays real-time system performance metrics, including CPU and memory usage with periodic updates.
    • Introduced a new dependency, psutil==7.0.0, to enhance system monitoring capabilities.

@coderabbitai
Copy link

coderabbitai bot commented Mar 12, 2025

Walkthrough

This update adds the psutil==7.0.0 dependency to both the environment.yml and requirements.txt files. Additionally, a new function monitor_hardware is introduced in the code, which utilizes psutil to monitor and display CPU and RAM usage in a Streamlit application. The sidebar has been updated with an expandable "Resource Utilization" section that calls this monitoring function, and minor formatting adjustments have been made.

Changes

File(s) Summary of Changes
environment.yml, requirements.txt Added new dependency: psutil==7.0.0 to the dependency lists.
src/common/common.py Added import psutil and a new function monitor_hardware (decorated with @st.fragment(run_every=5)) to capture and display CPU/RAM usage; modified render_sidebar to include a resource utilization section.

Sequence Diagram(s)

sequenceDiagram
    participant U as User
    participant SA as Streamlit App
    participant RS as render_sidebar
    participant MH as monitor_hardware
    participant PS as psutil

    U->>SA: Launch application
    SA->>RS: Render sidebar
    RS->>MH: Invoke monitor_hardware (every 5 sec)
    MH->>PS: Request CPU & RAM metrics
    PS-->>MH: Return hardware data
    MH->>RS: Update sidebar display (text, progress, caption)
    RS->>U: Display updated hardware stats
Loading

Poem

Oh, what a hop, a bounce so keen,
I scurry through updates, quick and clean.
New lines of code make my day bright,
Monitoring CPU in the sidebar light.
With psutil at hand, I proudly sing,
Celebrating changes with a joyful spring!
🐰🌟


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 58de157 and 5c9c8fa.

📒 Files selected for processing (1)
  • src/common/common.py (4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: build-simple-app
  • GitHub Check: build-full-app
  • GitHub Check: build-openms
🔇 Additional comments (3)
src/common/common.py (3)

13-13: LGTM: New import for system monitoring

The psutil library import is appropriately added to support the new hardware monitoring feature. According to the PR summary, this dependency was also added to both environment.yml and requirements.txt files with version 7.0.0.


26-37: Great implementation of the hardware monitoring function

The function effectively uses psutil to track system resources and displays them with appropriate Streamlit components. The @st.fragment(run_every=5) decorator ensures updates every 5 seconds without requiring page refresh, providing a seamless user experience.

A few observations:

  • Using interval=None for CPU calculation is good as it returns immediately without blocking
  • The RAM calculation correctly uses available memory relative to total memory
  • Adding the timestamp provides useful context for when metrics were last updated

364-365: Well-placed resource utilization display

The Resource Utilization expander has been appropriately positioned below the Settings expander in the sidebar, with clear emoji-based labeling that matches the rest of the UI. This implementation aligns perfectly with the requested changes mentioned in the PR comments.

✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🔭 Outside diff range comments (1)
src/common/common.py (1)

226-230: 💡 Verification agent

🧩 Analysis chain

Fix the variable assignment issue reported by Pylint.

There's a potential issue where workspaces_dir might be used before it's assigned on line 231, as indicated by the pipeline failure.

Apply this fix to ensure the variable is properly initialized:

-            workspace_dir = '..'
+            workspaces_dir = '..'
             
         # Check if workspace logic is enabled
         if st.session_state.settings["enable_workspaces"]:

Let's verify this issue with a script:


🏁 Script executed:

#!/bin/bash
# Verify the variable naming consistency in the code
rg "workspace_dir" src/common/common.py
rg "workspaces_dir" src/common/common.py

Length of output: 1299


Fix variable naming for workspace settings consistency

  • In file src/common/common.py, update the variable assignment on line 226 from workspace_dir = '..' to workspaces_dir = '..' to ensure it matches the rest of the file.
  • This change resolves the potential issue flagged by Pylint where workspaces_dir might have been referenced before being initialized.
-            workspace_dir = '..'
+            workspaces_dir = '..'
🧰 Tools
🪛 Ruff (0.8.2)

226-226: Local variable workspace_dir is assigned to but never used

Remove assignment to unused variable workspace_dir

(F841)

🧹 Nitpick comments (2)
environment.yml (1)

18-18: Dependency added correctly.

The psutil dependency has been added at the specific version 7.0.0, which is consistent with the addition in requirements.txt.

Consider adding a newline at the end of the file to satisfy linting rules:

        - psutil==7.0.0
+
🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 18-18: no new line character at the end of file

(new-line-at-end-of-file)

src/common/common.py (1)

26-37: Good implementation of hardware monitoring feature.

The function implements CPU and RAM visualization with appropriate refresh rate. The use of @st.fragment(run_every=5) is an excellent choice for this type of monitoring feature, as it allows updating the UI component without requiring a full page refresh.

Two small improvements could be made:

  1. Consider adding a docstring to explain the function's purpose
  2. Consider rounding the RAM percentage for better readability
@st.fragment(run_every=5)
def monitor_hardware():
+    """
+    Monitors and displays CPU and RAM usage statistics.
+    This function is automatically refreshed every 5 seconds.
+    """
    cpu_progress = psutil.cpu_percent(interval=None) / 100
-    ram_progress = 1 - psutil.virtual_memory().available / psutil.virtual_memory().total
+    memory = psutil.virtual_memory()
+    ram_progress = memory.percent / 100

    st.text(f"Ram ({ram_progress * 100:.2f}%)")
    st.progress(ram_progress)

    st.text(f"CPU ({cpu_progress * 100:.2f}%)")
    st.progress(cpu_progress)

    st.caption(f"Last fetched at: {time.strftime('%H:%M:%S')}")
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c78b10e and 09ce809.

📒 Files selected for processing (3)
  • environment.yml (1 hunks)
  • requirements.txt (1 hunks)
  • src/common/common.py (4 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
environment.yml

[error] 18-18: no new line character at the end of file

(new-line-at-end-of-file)

🪛 GitHub Actions: Pylint
src/common/common.py

[error] 231-231: E0606: Possibly using variable 'workspaces_dir' before assignment (possibly-used-before-assignment)

⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: build-simple-app
  • GitHub Check: build-full-app
  • GitHub Check: build-openms
🔇 Additional comments (4)
requirements.txt (1)

11-11: Dependency added correctly.

The psutil dependency has been added at version 7.0.0, which is consistent with the addition in environment.yml.

src/common/common.py (3)

13-13: Import added for new functionality.

The psutil library import is correctly added to support the new hardware monitoring feature.


294-296: Good integration of the monitoring feature in the sidebar.

The CPU and RAM visualization has been cleanly integrated into the sidebar with an appropriate expandable section.


365-365: Minor formatting adjustment.

The whitespace change around the session state assignment is minor and doesn't affect functionality.

@t0mdavid-m t0mdavid-m self-requested a review March 12, 2025 15:49
Copy link
Member

@t0mdavid-m t0mdavid-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Just two minor things: Could you rename the expander to "Resource Utilization" and move it below the settings expander? After this the PR is ready to merge from my side.

@subCode321
Copy link
Contributor Author

Looks good to me! Just two minor things: Could you rename the expander to "Resource Utilization" and move it below the settings expander? After this the PR is ready to merge from my side.

Sure!

@subCode321
Copy link
Contributor Author

Looks good to me! Just two minor things: Could you rename the expander to "Resource Utilization" and move it below the settings expander? After this the PR is ready to merge from my side.

image

Done! Moved Resource Utilisation below Settings

@subCode321 subCode321 requested a review from t0mdavid-m March 12, 2025 16:20
@t0mdavid-m
Copy link
Member

LGTM! Thank you for your contribution.

@t0mdavid-m t0mdavid-m merged commit 887157c into OpenMS:main Mar 13, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Visualization of Computational Ressources

2 participants