Skip to content

Enable pylint#1352

Merged
hmstepanek merged 19 commits intomainfrom
fix-linter-pylint
Apr 21, 2025
Merged

Enable pylint#1352
hmstepanek merged 19 commits intomainfrom
fix-linter-pylint

Conversation

@TimPansino
Copy link
Contributor

@TimPansino TimPansino commented Apr 11, 2025

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
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

@mergify mergify bot added the tests-failing Tests failing in CI. label Apr 11, 2025
@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 Tests failing in CI. label Apr 11, 2025
@hmstepanek hmstepanek self-requested a review April 15, 2025 03:30
@mergify mergify bot added the merge-conflicts Merge conflicts detected. label Apr 17, 2025
@mergify mergify bot removed the merge-conflicts Merge conflicts detected. label Apr 17, 2025
hmstepanek
hmstepanek reviewed Apr 17, 2025
View reviewed changes
newrelic/common/system_info.py
"""

global _nr_cached_hostname
global _nr_cached_hostname_lock
Copy link
Contributor

@hmstepanek hmstepanek Apr 17, 2025

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

@TimPansino TimPansino Apr 21, 2025

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
)
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

@hmstepanek hmstepanek Apr 18, 2025

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

@TimPansino TimPansino Apr 21, 2025

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


def cat_response_handler(self):
global CAT_RESPONSE_CODE
Copy link
Contributor

@hmstepanek hmstepanek Apr 18, 2025

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

@TimPansino TimPansino Apr 21, 2025

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