Skip to content

Add DynamicTimeoutFunction #383

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

Closed
wants to merge 1 commit into from
Closed

Add DynamicTimeoutFunction #383

wants to merge 1 commit into from

Conversation

Pijukatel
Copy link
Contributor

Add DynamicTimeoutFunction to enable custom timeouts for individual requests.

This allows users of ApifyClientAsync and ApifyClient to pass optional argument get_dynamic_timeout which is a function that can get suitable timeout for http requests to Apify API. Clients will use this function to set timeout for the requests.
Clients behave the same as before if the optional argument is not used.

Enables fix of: apify/crawlee-python#1132

@github-actions github-actions bot added this to the 112nd sprint - Tooling team milestone Apr 8, 2025
@github-actions github-actions bot added t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics. labels Apr 8, 2025
@Pijukatel Pijukatel requested a review from Copilot April 8, 2025 13:42
Copy link

@Copilot 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.

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.


For POST on endpoint v2/datasets/whatever/items timeout is proportional to the size of the content.
For everything else return fixed 30."""
if isinstance(content, bytes) and method == 'POST' and url.endswith('v2/datasets/whatever/items'):
Copy link
Preview

Copilot AI Apr 8, 2025

Choose a reason for hiding this comment

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

The dynamic timeout function only handles content that is of type bytes, but the tests pass strings (e.g., 'abcd'). To ensure the expected custom timeouts (e.g., 5, 9), update the type check or convert the content to bytes.

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

('content', 'expected_timeout'),
[
pytest.param('abcd', 5, id='Small payload'),
pytest.param('abcd' * 10000, 9, id='Payload in the dynamic timeout interval interval'),
Copy link
Preview

Copilot AI Apr 8, 2025

Choose a reason for hiding this comment

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

[nitpick] The test case id contains a repeated phrase ('interval interval'); consider revising it to remove the duplication.

Suggested change
pytest.param('abcd' * 10000, 9, id='Payload in the dynamic timeout interval interval'),
pytest.param('abcd' * 10000, 9, id='Payload in the dynamic timeout interval'),

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

@Pijukatel
Copy link
Contributor Author

Close din favor of: #384

@Pijukatel Pijukatel closed this Apr 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant