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

Make Azure op timeout an attribute #539

Merged
merged 2 commits into from
Aug 22, 2023

Conversation

skottmckay
Copy link
Contributor

Make timeout an attribute with default if not specified.
Set default to 15s (arbitrary - can change if there's a preferred value).

Update test model to validate timeout is read correctly.
@skottmckay skottmckay requested a review from a team as a code owner August 22, 2023 01:01
@@ -66,6 +66,7 @@ def make_graph(*args, doc_string=None, **kwargs):
audio_format='wav', # default audio type if filename is not specified.
model_uri='https://api.openai.com/v1/audio/transcriptions',
model_name='whisper-1',
timeout_seconds=20,
Copy link
Member

Choose a reason for hiding this comment

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

would you consider to make time_out be an input, so it may let the user set time-out on-the-fly according to the different network?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can consider it. I'd like to keep things simple until we figure out what exactly is required though.

the attribute provides a default - easily viewed but set at model creation time.
we can also make the custom op look for an input with the same suffix as the attribute providing a runtime value (this is already supported - something like 'nodeX/timeout_seconds' provides the 'timeout_seconds' value so nodes with the same custom op can have different timeouts).
that input can also be backed by an initializer with the same name (makes it an optional graph input) so the value is configurable but not mandatory to be provided by the user.

@skottmckay skottmckay merged commit eb5aef3 into main Aug 22, 2023
38 checks passed
@skottmckay skottmckay deleted the skottmckay/MakeTimeoutConfigurable branch August 22, 2023 22:21
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.

4 participants