Skip to content
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.

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

Added new feature for request timeout plugin #9525

Closed
wants to merge 4 commits into from

Conversation

joel6948
Copy link

@joel6948 joel6948 commented Apr 1, 2024

Overview
The purpose of this feature is to enhance Pylint's capabilities to detect missing timeouts for requests.Session() calls in Python code. Currently, Pylint's missing-timeout warning (W3101) is triggered for requests.get() calls but not for requests.Session(). This enhancement ensures consistent timeout handling across different usage patterns of the requests library.

Motivation
Timeout settings are crucial for robust network operations, especially in scenarios where network requests may encounter delays or failures. While Pylint already provides warnings for missing timeouts in requests.get() calls, similar warnings for requests.Session() calls are equally important to maintain code quality and reliability.

Implementation Details
Custom Pylint Plugin
A custom Pylint plugin (requests_timeout_plugin.py) is introduced to extend Pylint's functionality. This plugin inspects requests.Session() calls and raises a warning if no timeout is explicitly set.

Warning Message
The new warning message (W3102) is introduced to indicate the absence of a timeout setting for requests.Session() objects. This warning prompts developers to review and add appropriate timeout settings to ensure robust network operations.

Integration with Pylint
To use this feature, developers need to run Pylint with the requests_timeout_plugin.py plugin. They can specify the plugin using the --load-plugins option when invoking Pylint.

Example
Consider the following Python code snippet:
import requests

Without timeout setting

session = requests.Session()
response = session.get('https://example.com')

With the custom Pylint plugin enabled, Pylint will raise a warning (W3102) for the requests.Session() call, indicating that no timeout is set.

also you can use the plugin like
pylint --load-plugins=pylint_plugins.requests_timeout_plugin <your_code.py>

@joel6948
Copy link
Author

joel6948 commented Apr 1, 2024

#9517 this is the issue which was raised

Copy link
Contributor

github-actions bot commented Apr 1, 2024

🤖 Effect of this PR on checked open source code: 🤖

Effect on home-assistant:
The following messages are now emitted:

  1. missing-timeout-session:
    No timeout set for requests.Session() object.
    https://github.com/home-assistant/core/blob/71a6653f6021690d6dfaf3925cc0b10af0ba3195/homeassistant/components/tomato/device_tracker.py#L103
  2. missing-timeout-session:
    No timeout set for requests.Session() object.
    https://github.com/home-assistant/core/blob/71a6653f6021690d6dfaf3925cc0b10af0ba3195/homeassistant/components/tomato/device_tracker.py#L107
  3. missing-timeout-session:
    No timeout set for requests.Session() object.
    https://github.com/home-assistant/core/blob/71a6653f6021690d6dfaf3925cc0b10af0ba3195/homeassistant/components/huawei_lte/utils.py#L40
  4. missing-timeout-session:
    No timeout set for requests.Session() object.
    https://github.com/home-assistant/core/blob/71a6653f6021690d6dfaf3925cc0b10af0ba3195/homeassistant/components/google_wifi/sensor.py#L182

Effect on sentry:
The following messages are now emitted:

  1. missing-timeout-session:
    No timeout set for requests.Session() object.
    https://github.com/getsentry/sentry/blob/72f6e06d0aff1f2742da5c21102bb569b6a52887/src/sentry/services/hybrid_cloud/rpc.py#L581
  2. missing-timeout-session:
    No timeout set for requests.Session() object.
    https://github.com/getsentry/sentry/blob/72f6e06d0aff1f2742da5c21102bb569b6a52887/src/sentry/api/endpoints/project_app_store_connect_credentials.py#L170
  3. missing-timeout-session:
    No timeout set for requests.Session() object.
    https://github.com/getsentry/sentry/blob/72f6e06d0aff1f2742da5c21102bb569b6a52887/src/sentry/api/endpoints/project_app_store_connect_credentials.py#L462
  4. missing-timeout-session:
    No timeout set for requests.Session() object.
    https://github.com/getsentry/sentry/blob/72f6e06d0aff1f2742da5c21102bb569b6a52887/src/sentry/lang/native/appconnect.py#L224

This comment was generated for commit 391e70a

@Pierre-Sassoulas
Copy link
Member

Closing in favor of #9526

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.

2 participants