Skip to content

Bugfix // Streaming endpoint changes to match specification#48

Closed
stuartjohnpage wants to merge 3 commits intomainfrom
38-fix-new-line-streaming
Closed

Bugfix // Streaming endpoint changes to match specification#48
stuartjohnpage wants to merge 3 commits intomainfrom
38-fix-new-line-streaming

Conversation

@stuartjohnpage
Copy link

We recently fixed the prodops API to stream correctly according to the SSE.

This should now handle the splitting of those properly sent new lines correctly.

Comment on lines +66 to +73
|> String.split("\n\n")
|> Enum.map(fn event ->
event
|> String.split("\n")
|> Enum.filter(fn line -> String.starts_with?(line, "data: ") end)
|> Enum.map_join("\n", fn line -> String.trim_leading(line, "data: ") end)
end)
|> Enum.filter(fn data -> data != "" end)
Copy link

Choose a reason for hiding this comment

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

I'm not following how this fixes the problem... not saying it doesn't, I'm just confused. 🫤 Seems like it'd end up being the same in the end since we're splitting on the \n in the Enum.map. is the map_join adding back the newlines that were previously split?

Copy link
Author

@stuartjohnpage stuartjohnpage Apr 8, 2025

Choose a reason for hiding this comment

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

ye, exactly!

In the old code, you'd get a flat list of independent lines, losing the relationship between lines that were part of the same event.

eg:

data: Line 1
data: Line 2

data: Another event

originally would return: ["Line 1", "Line 2", "Another event"]

but now returns: ["Line 1\nLine 2", "Another event"]

The difference is that in the new approach, lines within the same event stay together with their formatting preserved

we have to split on the \n\n (the event delimited). Then we have each event. Then we want to grab the data bits, while preserving the new lines

Copy link

Choose a reason for hiding this comment

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

ah ha! thank you 🙏

pjdiiori
pjdiiori previously approved these changes Apr 8, 2025
@stuartjohnpage
Copy link
Author

I'm actually going to wrap this up when I make the new SDK for Revelry.AI 👍

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