-
Notifications
You must be signed in to change notification settings - Fork 4
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
Allow SDK methods to accept request_timeout
through kwargs
#332
base: main
Are you sure you want to change the base?
Conversation
def get_detector_by_name(self, name: str) -> Detector: | ||
# TODO should methods like this (which make direct requests instead of going through the API) allow | ||
# kwargs to be passed through? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's one type of tricky case for allowing request_timeout
to be specified.
@@ -960,6 +986,9 @@ def wait_for_confident_result( | |||
confidence_threshold: Optional[float] = None, | |||
timeout_sec: float = 30.0, | |||
) -> ImageQuery: | |||
# TODO should this method allow request_timeout to be passed in kwargs? | |||
# It's a weird case because it might make multiple requests - would the specified request_timeout be applied | |||
# to each request? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is another tricky case for accepting request_timeout
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An overall question: is there a sensible way to test this for all the methods we want to enable request_timeout
through kwargs for?
No description provided.