Improvements to HttpClient auto-configuration#115
Improvements to HttpClient auto-configuration#115ThomasVitale wants to merge 1 commit intolangchain4j:mainfrom
Conversation
Introduce shared HttpClientAutoConfiguration to define a HttpClientBuilder prototype bean and related task executors, with support for virtual threads (when on Java 21+) and context propagation (when Micrometer is used).
|
@dliubarskyi This is a suggestion for a possible improvement of the HTTP Client configuration, based on the ideas we considered in the previous PR. |
| private static final String EMBEDDING_MODEL_HTTP_CLIENT_BUILDER = "ollamaEmbeddingModelHttpClientBuilder"; | ||
|
|
||
| @Bean | ||
| @ConditionalOnMissingBean |
There was a problem hiding this comment.
This annotation is important because it allows developers to define their own OllamaChatModel bean if they need additional customisations.
Same for the other model beans in this class.
There was a problem hiding this comment.
Thanks! I guess ollamaStreamingLanguageModel should also be annotated with @ConditionalOnMissingBean?
...oconfigure/src/main/java/dev/langchain4j/autoconfigure/http/HttpClientAutoConfiguration.java
Show resolved
Hide resolved
| ObjectProvider<HttpClientBuilderCustomizer> customizers | ||
| ) { | ||
| HttpClientBuilder httpClientBuilder = new SpringRestClientBuilder() | ||
| .streamingRequestExecutor(asyncTaskExecutor) |
There was a problem hiding this comment.
Since there's always an AsyncTaskExecutor bean available, it can be configured directly, removing the need to operate the createDefaultStreamingRequestExecutor switch.
There was a problem hiding this comment.
Is it always available because of the bean definitions below (httpClientVirtualThreadsTaskExecutor, etc), right?
There was a problem hiding this comment.
I am wondering if there is a way to avoid creating an executor in case it is not required? Or it is not a concern worth worrying about?
...oconfigure/src/main/java/dev/langchain4j/autoconfigure/http/HttpClientBuilderCustomizer.java
Show resolved
Hide resolved
dliubarskyi
left a comment
dliubarskyi
left a comment
There was a problem hiding this comment.
@ThomasVitale thanks a lot, this looks great!
I like how it simplifies the code, but I am a bit worried that now there is no way to precisely configure client/executor per model type and/or provider. WDYT?
| return SpringRestClient.builder() | ||
| .restClientBuilder(restClientBuilder.getIfAvailable(RestClient::builder)) | ||
| // executor is not needed for no-streaming OllamaChatModel | ||
| .createDefaultStreamingRequestExecutor(false); |
There was a problem hiding this comment.
If I understand correctly, with current configuration, task executor will always be created, even if it is not required (e.g., only non-streaming OllamaChatModel is used in the application)?
| private static final String EMBEDDING_MODEL_HTTP_CLIENT_BUILDER = "ollamaEmbeddingModelHttpClientBuilder"; | ||
|
|
||
| @Bean | ||
| @ConditionalOnMissingBean |
There was a problem hiding this comment.
Thanks! I guess ollamaStreamingLanguageModel should also be annotated with @ConditionalOnMissingBean?
| @ConditionalOnProperty(PREFIX + ".chat-model.base-url") | ||
| OllamaChatModel ollamaChatModel( | ||
| @Qualifier(CHAT_MODEL_HTTP_CLIENT_BUILDER) HttpClientBuilder httpClientBuilder, | ||
| HttpClientBuilder httpClientBuilder, |
There was a problem hiding this comment.
Do I understand correctly that there is currently no way to customize http client and/or executor per model type (e.g., use different configuration for non-streaming and streaming model) or provider (e.g., use different configuration for different LLM providers)?
|
|
||
| <dependency> | ||
| <groupId>dev.langchain4j</groupId> | ||
| <artifactId>langchain4j-spring-boot-autoconfigure</artifactId> |
There was a problem hiding this comment.
(note for myself): need to do the same for OpenAI SB starter
| ObjectProvider<HttpClientBuilderCustomizer> customizers | ||
| ) { | ||
| HttpClientBuilder httpClientBuilder = new SpringRestClientBuilder() | ||
| .streamingRequestExecutor(asyncTaskExecutor) |
There was a problem hiding this comment.
Is it always available because of the bean definitions below (httpClientVirtualThreadsTaskExecutor, etc), right?
| } | ||
|
|
||
| @Test | ||
| void httpClientBuilderWhenNoAutoConfiguredRestClient() { |
There was a problem hiding this comment.
Could you please explain when/why can this happen that there is no RestClientAutoConfiguration?
| ObjectProvider<HttpClientBuilderCustomizer> customizers | ||
| ) { | ||
| HttpClientBuilder httpClientBuilder = new SpringRestClientBuilder() | ||
| .streamingRequestExecutor(asyncTaskExecutor) |
There was a problem hiding this comment.
I am wondering if there is a way to avoid creating an executor in case it is not required? Or it is not a concern worth worrying about?
2 participants
Change
Introduce shared
HttpClientAutoConfigurationto define aHttpClientBuilderprototype bean and related task executors, with support for virtual threads (when on Java 21+) and context propagation (when Micrometer is used).General checklist