Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Sep 3, 2024

Description

This command fixes the CPU and RAM usage that is shown in $ping. Instead of reading the stats from the whole system, it reads the usage of the current PID.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Other: (write here)

Guidelines

  • My code follows the style guidelines of this project (formatted with Ruff)

  • I have performed a self-review of my own code

  • I have commented my code, particularly in hard-to-understand areas

  • I have made corresponding changes to the documentation if needed

  • My changes generate no new warnings

  • I have tested this change

  • Any dependent changes have been merged and published in downstream modules

  • I have followed all of these guidelines.

How Has This Been Tested? (if applicable)

Tested in Tux dev server

Summary by Sourcery

Refactor the Ping command to accurately report CPU and RAM usage by focusing on the current process rather than the entire system.

Bug Fixes:

  • Fix CPU and RAM usage calculation in the Ping command to reflect the current process instead of the entire system.

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Sep 3, 2024

Reviewer's Guide by Sourcery

This pull request refactors the Ping command to fix CPU and RAM usage reporting. The changes focus on reading stats for the current process instead of the whole system, providing more accurate resource usage information for the bot.

File-Level Changes

Change Details Files
Modify CPU usage calculation to use process-specific data
  • Replace psutil.cpu_percent() with psutil.Process().cpu_percent()
  • This change ensures that only the CPU usage of the bot process is reported
tux/cogs/utility/ping.py
Update RAM usage calculation to use process-specific data
  • Replace psutil.virtual_memory().used with psutil.Process().memory_info().rss
  • Convert RAM usage from bytes to megabytes
  • Update the formatting logic to handle the new megabytes-based calculation
tux/cogs/utility/ping.py
Improve RAM usage formatting
  • Modify the condition for GB/MB formatting to use the new megabytes-based value
  • Update the formatting strings to use floating-point precision for more accurate reporting
tux/cogs/utility/ping.py

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @wlinator - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

@ghost ghost changed the title refactor: Fix the CPU and RAM usage in the Ping command Fix the CPU and RAM usage in the Ping command Sep 3, 2024
@kzndotsh kzndotsh merged commit 2f5ffee into main Sep 3, 2024
@kzndotsh kzndotsh deleted the tess-fixping branch September 3, 2024 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant