Skip to content

OpenAI: investigation for StreamingClientResult<T>#39

Draft
annelo-msft wants to merge 2 commits intojoseharriaga:mainfrom
annelo-msft:openai-streaming-result
Draft

OpenAI: investigation for StreamingClientResult<T>#39
annelo-msft wants to merge 2 commits intojoseharriaga:mainfrom
annelo-msft:openai-streaming-result

Conversation

@annelo-msft
Copy link
Copy Markdown
Collaborator

Depends on this change to ClientModel: Azure/azure-sdk-for-net#42873

public class StreamingClientResult<T>
: IDisposable
, IAsyncEnumerable<T>
internal class StreamingEventResult<T> : StreamingClientResult<T>
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This type becomes internal.

_disposedValue = true;
}
}
// TODO: Handle disposal via Enumerator? Validate that this will work.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is an open question we'll still need to resolve - whether or not to make StreamingClientResult implement IDisposable.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

From the earlier-era Toub discussions, I think we'll end up needing IDisposable (or have a variant, equivalent mechanism, or whatever else). We looked at the option of implicitly using the end of enumeration as the sole disposal mechanism (and the existing implementation does opportunistically do it at the end of enumeration), but the ergonomics around making that an explicit lifetime requirement were unfavorably received. I think Krzysztof also mentioned that leaving an unbuffered response floating was a substantial issue.

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.

2 participants