Skip to content

Enable pylint #1352

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 19 commits into from
Apr 21, 2025
Merged

Enable pylint #1352

merged 19 commits into from
Apr 21, 2025

Conversation

TimPansino
Copy link
Contributor

Overview

  • Enable pylint rules in ruff.
    • Disable pylint recommendations for now, potentially re-enable later.
    • Disable rules for global statements and redefined loop variables.
      • Too many of these to tackle at once, and most are intentional design choices for better or worse.
    • Fix all other pylint rule violations.

@TimPansino TimPansino requested a review from a team as a code owner April 11, 2025 21:04
@TimPansino TimPansino changed the title Fix linter pylint Enable pylint Apr 11, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 11, 2025 •
edited
Loading

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Warnings Elapsed time
✅ ACTION actionlint 5 0 0 0.46s
✅ MARKDOWN markdownlint 7 0 0 0 1.03s
✅ MARKDOWN markdown-link-check 7 0 0 16.72s
✅ PYTHON ruff 893 0 0 0 0.57s
✅ PYTHON ruff-format 893 0 0 0 0.2s
✅ YAML prettier 12 0 0 0 1.07s
✅ YAML v8r 12 0 0 5.69s
✅ YAML yamllint 12 0 0 0.54s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

@codecov-commenter
Copy link

codecov-commenter commented Apr 11, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 79.59184% with 10 lines in your changes missing coverage. Please review.

Project coverage is 81.06%. Comparing base (c0d9da9) to head (f4f78f8).
Report is 24 commits behind head on main.

Files with missing lines Patch % Lines
newrelic/hooks/external_botocore.py 25.00% 0 Missing and 3 partials ⚠️
newrelic/common/streaming_utils.py 0.00% 1 Missing and 1 partial ⚠️
newrelic/api/external_trace.py 50.00% 0 Missing and 1 partial ⚠️
newrelic/api/function_trace.py 50.00% 0 Missing and 1 partial ⚠️
newrelic/api/web_transaction.py 0.00% 1 Missing ⚠️
newrelic/hooks/messagebroker_confluentkafka.py 50.00% 1 Missing ⚠️
newrelic/hooks/messagebroker_kombu.py 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1352      +/-   ##
==========================================
- Coverage   81.07%   81.06%   -0.02%     
==========================================
  Files         204      204              
  Lines       22830    22832       +2     
  Branches     3618     3618              
==========================================
- Hits        18510    18508       -2     
- Misses       3120     3123       +3     
- Partials     1200     1201       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mergify mergify bot removed the tests-failing label Apr 11, 2025
@hmstepanek hmstepanek self-requested a review April 15, 2025 03:30
@mergify mergify bot removed the merge-conflicts label Apr 17, 2025
hmstepanek
hmstepanek reviewed Apr 17, 2025
View reviewed changes
newrelic/common/system_info.py
@@ -349,7 +349,6 @@ def gethostname(use_dyno_names=False, dyno_shorten_prefixes=()):
"""

global _nr_cached_hostname
global _nr_cached_hostname_lock
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PLW0602 - global-variable-not-assigned

There's no need to make this variable global when it's never assigned to. It already reads the globally scoped variable when attempting to use the lock.

hmstepanek
hmstepanek reviewed Apr 18, 2025
View reviewed changes
tests/agent_unittests/test_agent_protocol.py
@@ -363,7 +360,7 @@ def connect_payload_asserts(
],
)
def test_connect(with_aws, with_ecs, with_pcf, with_gcp, with_azure, with_docker, with_kubernetes, with_ip):
global AWS, AZURE, GCP, PCF, BOOT_ID, DOCKER, KUBERNETES, IP_ADDRESS
global AWS, AZURE, GCP, PCF, DOCKER, KUBERNETES, IP_ADDRESS
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was BOOT_ID dropped from being a global?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also PLW0602 - global-variable-not-assigned

tests/external_httpx/test_client.py
@@ -36,7 +36,6 @@


def cat_response_handler(self):
global CAT_RESPONSE_CODE
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question as the others about this global.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also PLW0602 - global-variable-not-assigned

@TimPansino @hmstepanek
Apply suggestions from code review
4ac3f5a 
Co-authored-by: Hannah Stepanek <hstepanek@newrelic.com>
@TimPansino TimPansino requested a review from hmstepanek April 21, 2025 18:29
@hmstepanek hmstepanek enabled auto-merge April 21, 2025 19:53
@hmstepanek hmstepanek merged commit a7fb3af into main Apr 21, 2025
54 of 57 checks passed
@hmstepanek hmstepanek deleted the fix-linter-pylint branch April 21, 2025 20:11
@TimPansino TimPansino added this to the v10.10.0 milestone Apr 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hmstepanek hmstepanek hmstepanek approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v10.10.0
Development

Successfully merging this pull request may close these issues.
3 participants
@TimPansino @codecov-commenter @hmstepanek