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

switch to '\n' for EventSourceResponse separator #1188

Merged
merged 2 commits into from
Feb 15, 2024

Conversation

khimaros
Copy link
Contributor

@khimaros khimaros commented Feb 14, 2024

this fixes compatibility with some OpenAI clients, including BetterChatGPT (ztjhz/BetterChatGPT#537). i'm not sure if this will cause problems with other clients. i'm also working with the client to see if they are willing to handle these separators more gracefully.

this fixes compatibility with some OpenAI clients, including BetterChatGPT (ztjhz/BetterChatGPT#537).
@khimaros
Copy link
Contributor Author

note that BetterChatGPT client still has trouble with sse-starlet ping messages as described in ztjhz/BetterChatGPT#538 -- the hacky workaround i'm using locally for this is to set the ping=100000 on the EventSourceResponse -- sse-starlet doesn't seem to provide a way to disable these entirely so it seems like a client side solution would be ideal.

@abetlen
Copy link
Owner

abetlen commented Feb 15, 2024

@khimaros makes sense, I'm assuming the \n seperator is what's used by the openai api when doing server sent events / streaming.

@abetlen
Copy link
Owner

abetlen commented Feb 15, 2024

Thank you, I'll go ahead and merge this.

@abetlen abetlen merged commit ea1f88d into abetlen:main Feb 15, 2024
16 checks passed
@khimaros khimaros deleted the patch-1 branch March 5, 2024 21:41
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.

None yet

2 participants