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

Users want to have control over the request timeout #118

Open
6 tasks done
rhamzeh opened this issue Aug 7, 2024 · 5 comments
Open
6 tasks done

Users want to have control over the request timeout #118

rhamzeh opened this issue Aug 7, 2024 · 5 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@rhamzeh
Copy link
Member

rhamzeh commented Aug 7, 2024

Checklist

  • I have looked into the README and have not found a suitable solution or answer.
  • I have looked into the documentation and have not found a suitable solution or answer.
  • I have searched the issues and have not found a suitable solution or answer.
  • I have upgraded to the latest version of OpenFGA and the issue still persists.
  • I have searched the Slack community and have not found a suitable solution or answer.
  • I agree to the terms within the OpenFGA Code of Conduct.

Description

OpenFgaApi methods support passing in a _request_timeout:

:param _request_timeout: timeout setting for this request. If one
number provided, it will be total request
timeout. It can also be a pair (tuple) of
(connection, read) timeouts.

However the options_to_kwargs

def options_to_kwargs(options: dict[str, int | str] = None):
"""
Return kwargs with continuation_token and page_size
"""
kwargs = {}
if options is not None:
if options.get("page_size"):
kwargs["page_size"] = options["page_size"]
if options.get("continuation_token"):
kwargs["continuation_token"] = options["continuation_token"]
if options.get("headers"):
kwargs["_headers"] = options["headers"]
if options.get("retry_params"):
kwargs["_retry_params"] = options["retry_params"]
return kwargs
does not pass in that option down from the OpenFgaClient methods to the OpenFgaApi methods

Expectation

A user should be able to set the request timeout on a method.

Note that instead of just allowing passing in _request_timeout per method, this should be done like in the Java SDK.

Users should be able to configure a default timeout in the Configuration/ClientConfiguration, and override it per method.

Example in Java: https://github.com/openfga/java-sdk/blob/b1e03e523c530824f5921313c2191dc5f6d93af8/src/main/java/dev/openfga/sdk/api/configuration/BaseConfiguration.java#L22-L24

Reproduction

.

OpenFGA SDK version

v0.6.1

OpenFGA version

N/A

SDK Configuration

N/A

Logs

No response

References

https://cloud-native.slack.com/archives/C06G1NNH47N/p1723022081318009

@GMorris-professional
Copy link

GMorris-professional commented Sep 19, 2024

I'd like to pick this up 😀 Nevermind I see there is a PR for this already 👍

@rhamzeh
Copy link
Member Author

rhamzeh commented Oct 28, 2024

@Oscmage you have a draft PR for this - are you still working on it?

If not, would you mind if @ryanpq steps in to help take it to completion?

@Oscmage
Copy link

Oscmage commented Oct 29, 2024

Please be my guest!

@ryanpq
Copy link
Contributor

ryanpq commented Nov 1, 2024

I've revised the updates in the PR to use milliseconds as advised for consistency across sdks while still passing second values to requests and urllib3 as they require.

@Oscmage
Copy link

Oscmage commented Nov 4, 2024

@ryanpq Thanks a lot for taking this all the way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
Status: Backlog
Development

No branches or pull requests

4 participants