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

Use httr2::req_perform_stream(round = "line") #65

Merged

Conversation

romainfrancois
Copy link
Contributor

@romainfrancois romainfrancois commented Feb 8, 2024

closes #63

This uses httr2::req_perform_stream_lines() from r-lib/httr2#437 so that the data from open ai is dealt with line by line instead of some number of bytes.

This PR is meant more as a conversation aid for the httr2 PR.

If httr2 actually had httr2::req_perform_stream_lines() or something serving the same purpose: consume stream line by line, then I believe some of the logic around ch_env$stream$raw could be improved, because we would know that we get complete data: {json} lines, and so openai_stream_parse() etc ... could be simplified.

Compared to #64 which has to do tricks with the bytes buffer, this feels much nicer.

So now we can stream the golem poem, cc @ColinFay 🫣

Screen.Recording.2024-02-08.at.11.58.50.mov

@romainfrancois romainfrancois marked this pull request as ready for review February 15, 2024 05:24
@romainfrancois romainfrancois changed the title Use httr2::req_perform_stream_lines() Use httr2::req_perform_stream(round = "line") Feb 15, 2024
@edgararuiz edgararuiz merged commit fb75dfd into mlverse:main Feb 20, 2024
6 checks passed
@edgararuiz
Copy link
Collaborator

Thanks @romainfrancois !

@romainfrancois romainfrancois deleted the use_httr2_req_perform_stream_lines branch February 20, 2024 16:35
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.

truncated openai_stream_parse(x=) : .p() must return a single TRUE or FALSE, not NA
2 participants