-
Notifications
You must be signed in to change notification settings - Fork 62
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
feat: β¨ ollama streaming mode #522
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the start!
I added some comments
...a/deployment/src/main/java/io/quarkiverse/langchain4j/ollama/deployment/OllamaProcessor.java
Outdated
Show resolved
Hide resolved
...a/deployment/src/main/java/io/quarkiverse/langchain4j/ollama/deployment/OllamaProcessor.java
Outdated
Show resolved
Hide resolved
ollama/runtime/pom.xml
Outdated
@@ -26,7 +26,36 @@ | |||
<artifactId>quarkus-langchain4j-core</artifactId> | |||
<version>${project.version}</version> | |||
</dependency> | |||
|
|||
<dependency> | |||
<groupId>dev.langchain4j</groupId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I needed it for one of my tests, but it's no longer in use.
I've deleted it (and I'll see if I need some classes from this dependency later π).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
β‘οΈ 35a077b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@geoand after removed it i remember (or Maven remember to me π) why I've needed the add the dependency: it was to import the class dev.langchain4j.model.ollama.OllamaStreamingChatModel
.
If I well understood, for the supplier I need a builder and this builder is in this class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I confess that at this stage, I don't understand yet why, for some models, some classes are used from langchain4j and other rewritten with duplicated code.
Other thinks I need to understand, for example, is why for some models the langchain4j client is used or a new one is created (like Mistral for example).
I'm trying to understand the architecture of the code, and I think I'm going to make some mistakes, sorry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The general idea was that we try to reuse upstream bit, but for some integrations the upstream code is so mimimal that it just does not bring any benefit to do so.
Such is the case of the Ollama extension
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I begin to understand this and I agree with you π
Thanks, this is a nice start! The most important part now is the implementation of |
Yes, I've started the implementation, I need to correct one or two mistakes but it's coming along! |
@philippart-s I think that this link can help you to understand how to create the REST API call for the streaming. About the code implementation, you might want to take a look at what I did for the BAM module here and here (I hope this can help you). |
@geoand : I've a first version that I want to test but when I run the integration tests (for example https://github.com/quarkiverse/quarkus-langchain4j/tree/main/integration-tests/ollama, I've this error: I run the |
π I would first build the entire project and then do |
it's what I did: β cd quarkus-langchain4j/integration-tests/ollama
β mvn quarkus:dev
[INFO] Scanning for projects...
[INFO]
[INFO] --< io.quarkiverse.langchain4j:quarkus-langchain4j-integration-test-ollama >--
[INFO] Building Quarkus LangChain4j - Integration Tests - Ollama 999-SNAPSHOT
[INFO] from pom.xml
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- quarkus:3.8.2:dev (default-cli) @ quarkus-langchain4j-integration-test-ollama ---
[INFO] Invoking enforcer:3.4.1:enforce (enforce-java-version) @ quarkus-langchain4j-integration-test-ollama
[INFO] Rule 0: org.apache.maven.enforcer.rules.BannedRepositories passed
[INFO] Rule 1: org.apache.maven.enforcer.rules.version.RequireJavaVersion passed
[INFO] Invoking enforcer:3.4.1:enforce (enforce-maven-version) @ quarkus-langchain4j-integration-test-ollama
[INFO] Rule 0: org.apache.maven.enforcer.rules.version.RequireMavenVersion passed
[INFO] Invoking sundr:0.103.1:generate-bom (default) @ quarkus-langchain4j-integration-test-ollama
[INFO] Invoking buildnumber:3.2.0:create (get-scm-revision) @ quarkus-langchain4j-integration-test-ollama
[INFO] Executing: /bin/sh -c cd '/Users/stef/Dev/quarkus-langchain4j/integration-tests/ollama' && 'git' 'rev-parse' '--verify' 'HEAD'
[INFO] Working directory: /Users/stef/Dev/quarkus-langchain4j/integration-tests/ollama
[INFO] Storing buildNumber: efc0de2213cdc181d26d9377a46890c403b0642b at timestamp: 1714568697753
[INFO] Executing: /bin/sh -c cd '/Users/stef/Dev/quarkus-langchain4j/integration-tests/ollama' && 'git' 'symbolic-ref' 'HEAD'
[INFO] Working directory: /Users/stef/Dev/quarkus-langchain4j/integration-tests/ollama
[INFO] Storing scmBranch: ollama-streaming
[INFO] Invoking formatter:2.23.0:format (format-sources) @ quarkus-langchain4j-integration-test-ollama
[INFO] Processed 1 files in 259ms (Formatted: 0, Skipped: 1, Unchanged: 0, Failed: 0, Readonly: 0)
[INFO] Invoking impsort:1.9.0:sort (sort-imports) @ quarkus-langchain4j-integration-test-ollama
[INFO] Processed 1 files in 00:00.005 (Already Sorted: 1, Needed Sorting: 0)
[INFO] Invoking resources:3.3.1:resources (default-resources) @ quarkus-langchain4j-integration-test-ollama
[INFO] Copying 1 resource from src/main/resources to target/classes
[INFO] Invoking compiler:3.12.1:compile (default-compile) @ quarkus-langchain4j-integration-test-ollama
[INFO] Nothing to compile - all classes are up to date.
[INFO] Invoking resources:3.3.1:testResources (default-testResources) @ quarkus-langchain4j-integration-test-ollama
[INFO] skip non existing resourceDirectory /Users/stef/Dev/quarkus-langchain4j/integration-tests/ollama/src/test/resources
[INFO] Invoking compiler:3.12.1:testCompile (default-testCompile) @ quarkus-langchain4j-integration-test-ollama
[INFO] No sources to compile
[WARNING] [io.quarkus.bootstrap.devmode.DependenciesFilter] Live reload was disabled for the following project artifacts:
- io.quarkiverse.langchain4j:quarkus-langchain4j-ollama:999-SNAPSHOT
- io.quarkiverse.langchain4j:quarkus-langchain4j-core:999-SNAPSHOT
- io.quarkiverse.langchain4j:quarkus-langchain4j-core-runtime-spi:999-SNAPSHOT
- io.quarkiverse.langchain4j:quarkus-langchain4j-core-deployment:999-SNAPSHOT
- io.quarkiverse.langchain4j:quarkus-langchain4j-ollama-deployment:999-SNAPSHOT
The artifacts above appear to be either dependencies of non-reloadable application dependencies or Quarkus extensions
Listening for transport dt_socket at address: 5005
2024-05-01 15:05:00,120 INFO [io.qua.dep.dev.IsolatedDevModeMain] (main) Attempting to start live reload endpoint to recover from previous Quarkus startup failure
2024-05-01 15:05:00,335 ERROR [io.qua.dep.dev.IsolatedDevModeMain] (main) Failed to start quarkus: java.lang.IllegalStateException: No config found for interface io.quarkiverse.langchain4j.runtime.aiservice.ChatMemoryConfig
at io.quarkus.deployment.ExtensionLoader.loadStepsFrom(ExtensionLoader.java:186)
at io.quarkus.deployment.QuarkusAugmentor.run(QuarkusAugmentor.java:107)
at io.quarkus.runner.bootstrap.AugmentActionImpl.runAugment(AugmentActionImpl.java:330)
at io.quarkus.runner.bootstrap.AugmentActionImpl.createInitialRuntimeApplication(AugmentActionImpl.java:251)
at io.quarkus.runner.bootstrap.AugmentActionImpl.createInitialRuntimeApplication(AugmentActionImpl.java:60)
at io.quarkus.deployment.dev.IsolatedDevModeMain.firstStart(IsolatedDevModeMain.java:112)
at io.quarkus.deployment.dev.IsolatedDevModeMain.accept(IsolatedDevModeMain.java:433)
at io.quarkus.deployment.dev.IsolatedDevModeMain.accept(IsolatedDevModeMain.java:55)
at io.quarkus.bootstrap.app.CuratedApplication.runInCl(CuratedApplication.java:138)
at io.quarkus.bootstrap.app.CuratedApplication.runInAugmentClassLoader(CuratedApplication.java:93)
at io.quarkus.deployment.dev.DevModeMain.start(DevModeMain.java:131)
at io.quarkus.deployment.dev.DevModeMain.main(DevModeMain.java:62) I have this error for all tests in the |
Weird... I've never seen that happen and it works fine for me (and CI) |
I'll check again all my configuration to see if I have an issue on my local configuration. |
Would you like to push what you have so I can check it out locally? |
after a fresh build it's working π |
@geoand here is the first version with streaming mode.
I think I've reached the limit of my knowledge on how to call Ollama and how to integrate the streaming function into this extension. |
Thanks a lot @philippart-s! I will take your code and finish the implementation soon. |
Thanks, don't hesitate to ping me, I'll see what you fix in my code and be more autonomous the next time. |
No need to apologize! Thanks a lot for getting the ball rolling on this one! I will ping you when I've completed the PR :) |
Co-Authored-By: Georgios Andrianakis <geoand@gmail.com>
The problem with the exception you are seeing turns out to be weirder than I thought... All we can do for the time being is workaround it as I have done. Do you mind checking if things work for you with the latest version (I've force pushed to your branch so you'll have to be careful when pulling)? |
yes, I'm going to try it as soon as I have a bit of time in my day π |
@geoand I tested with my app and it seems ok π. Just for my information, for the next PR, what type of configuration for the code formatter is used? (to avoid the maven verify error on the CI π ) |
Thanks for checking! All I do is |
π |
PR to try to add the streaming feature to Ollama provider through the extension π
First commits are the setup of the the feature ποΈ.