Skip to content

Fix issue #244 #261

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 2 commits into from
Closed

Conversation

isaac-j-miller
Copy link

  • Don't dispose the client websocket in the AsyncWebsocketMessageEnumerator
  • Bump version suffix to beta.2 (not sure if this is correct)

@isaac-j-miller
Copy link
Author

isaac-j-miller commented Oct 18, 2024

Fixes #261

@isaac-j-miller isaac-j-miller marked this pull request as draft October 18, 2024 18:01
@isaac-j-miller isaac-j-miller marked this pull request as ready for review October 18, 2024 18:49
Copy link
Collaborator

@jsquire jsquire left a comment

Choose a reason for hiding this comment

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

Hi @isaac-j-miller. Thank you for your contribution and your interest in improving the OpenAI developer experience. I apologize that it took so long for someone to review this.

After looking over the associated issue and the RealtimeSession implementation, the session also manages the socket lifetime in its Dispose method, so we agree that transferring ownership to the enumerator is a mistake.

Unfortunately, the structure of the repo has changed because it took us so long to get to this, so it can't be merged as-is. Since #527 is open to make other changes in this area, I adopted the proposed change there. I'm going to close this out.

Thank you, again, for your efforts in putting this together!

@jsquire jsquire closed this Jul 10, 2025
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