Skip to content

Conversation

@goncalossilva
Copy link
Member

@goncalossilva goncalossilva commented Mar 31, 2025

These represent a good baseline for the vast majority of cases, and the remainder can customize the Session object.

We could provide ways to configure this, but seeing as the vast majority won't need it, I'd argue against complicating the SDK unnecessarily. As @rsyring said in #88, which I agree with:

However, I'd argue the far majority of people using this library won't even think about missing timeouts until it's a problem.

Special cases can customize the Session argument.

Closes #88.

@goncalossilva goncalossilva requested a review from lefcha March 31, 2025 17:26
@goncalossilva goncalossilva requested a review from a team as a code owner March 31, 2025 17:26
@goncalossilva goncalossilva requested a review from Copilot March 31, 2025 17:26
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR configures a baseline timeout for API requests in order to prevent hanging connections. It adds a TIMEOUT constant to the HTTP requests module, updates get, post, and delete methods to apply the timeout, and documents the change in the CHANGELOG.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
todoist_api_python/http_requests.py Adds a TIMEOUT constant and applies it to all session request methods.
CHANGELOG.md Updates the changelog to reflect the new API timeout configuration.

These represent a good baseline for 99.99% of cases.
The remainder can customize the `Session` argument.
@goncalossilva goncalossilva force-pushed the goncalossilva/default-request-timeouts branch from fce153d to c4aea1e Compare March 31, 2025 17:28
@goncalossilva goncalossilva enabled auto-merge March 31, 2025 17:28
@goncalossilva goncalossilva merged commit 65164c7 into main Mar 31, 2025
1 check passed
@goncalossilva goncalossilva deleted the goncalossilva/default-request-timeouts branch March 31, 2025 17:28
Copy link
Collaborator

@lefcha lefcha 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!

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.

Add default timeouts for http requests

3 participants