[Platform][Ollama] Support streaming output #339
Conversation
logicalor
requested review from
chr-hertel,
Nyholm and
OskarStark
as code owners
carsonbot
added
Bug
carsonbot
changed the title
logicalor
changed the title
|
Can you please add an example in examples/ directory? Thanks |
OskarStark
changed the title
@OskarStark updated with an example |
|
Thanks It looks like your committer email is not associated with your GitHub account |
@OskarStark thanks, hadn't noticed that. Added. |
| if ($msg) { | ||
| yield $msg; | ||
| } | ||
| } |
There was a problem hiding this comment.
Question for @chr-hertel (not relevant for the review), isn't the same situation that we're facing in #324?
There was a problem hiding this comment.
It's not since this is only working on the response in the ResultConverter, which is only called when finally fetching the content. In #324 the stream is called already in the client
| // Emit each chunk as it is received | ||
| foreach ($result as $chunk) { | ||
| echo $chunk->getContent(); | ||
| } |
There was a problem hiding this comment.
| } | |
| } | |
| echo \PHP_EOL; |
There was a problem hiding this comment.
Let's call the file only stream.php - it is more consistent with the other stream examples, and ollama is basically the folder already
|
which ollama version do you use? |
| $data = json_decode($json, true); | ||
| if (!\is_array($data)) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
please move this to the result converter - it's their job to convert to data. fine with having the value object, but let's keep it simple than 👍
There was a problem hiding this comment.
Thanks @logicalor for reporting and working on this - we need this :)
Made some comments, but i think all in all we're close to merging it 👍
Both a bug fix and a feature? Support streaming output from Ollama, preventing an error being thrown if streaming output is sent with the chat request.