Skip to content

Streamline dependencies and improve API consistency #1550

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

Closed
wants to merge 2 commits into from

Conversation

tzolov
Copy link
Contributor

@tzolov tzolov commented Oct 16, 2024

  • Remove unnecessary spring-web dependencies
  • Update third-party library versions
  • Refactor API classes to use consistent header handling
  • Remove ApiUtils class and inline its functionality
  • Adjust RestClient and WebClient builder usage in autoconfiguration
  • Replace direct RestClient.Builder injections with ObjectProvider<RestClient.Builder> and WebClient.Builder injections with ObjectProvider<WebClient.Builder>
  • Update ChromaVectorStoreAutoConfiguration to use ObjectProvider
  • Rename MongoDbAtlasLocalContainerConnectionDetailsFactoryTest to IT
  • Switch spring-ai-chroma-store dependency from spring-web to spring-webflux
  • Simplify ChromaApi constructor by using method reference for default headers

Resolves #1066, #524

@@ -99,15 +103,20 @@ public ZhiPuAiApi(String baseUrl, String zhiPuAiToken, RestClient.Builder restCl
*/
public ZhiPuAiApi(String baseUrl, String zhiPuAiToken, RestClient.Builder restClientBuilder, ResponseErrorHandler responseErrorHandler) {

Consumer<HttpHeaders> authHeaders = h -> {
h.setBearerAuth(zhiPuAiToken);
h.setContentType(MediaType.APPLICATION_JSON);
Copy link
Member

Choose a reason for hiding this comment

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

i'll run the IT to make sure adding content type is ok.

Copy link
Member

Choose a reason for hiding this comment

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

unfortunately, my api key no longer works. will have to follow up on it another time.

@@ -68,8 +70,13 @@ public ZhiPuAiImageApi(String baseUrl, String zhiPuAiToken, RestClient.Builder r
public ZhiPuAiImageApi(String baseUrl, String zhiPuAiToken, RestClient.Builder restClientBuilder,
ResponseErrorHandler responseErrorHandler) {

Consumer<HttpHeaders> authHeaders = h -> {
Copy link
Member

Choose a reason for hiding this comment

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

could be inlined as there is just one usgae.

@@ -34,6 +34,7 @@
<version>${spring-retry.version}</version>
</dependency>

<!-- -->
Copy link
Member

Choose a reason for hiding this comment

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

can remove extraneous comment.


return new AnthropicApi(connectionProperties.getBaseUrl(), connectionProperties.getApiKey(),
connectionProperties.getVersion(), restClientBuilder, webClientBuilder, responseErrorHandler,
connectionProperties.getVersion(), restClientBuilderProvider.getIfAvailable(RestClient::builder),
Copy link
Member

Choose a reason for hiding this comment

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

if unavailable, we should create it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In contrary we should. This is the purpose of using the ObjectProvider

Copy link
Member

Choose a reason for hiding this comment

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

my bad, i didn't realize that is how getIfAvailable worked!

 - Remove unnecessary spring-web dependencies
 - Update third-party library versions
 - Refactor API classes to use consistent header handling
 - Remove ApiUtils class and inline its functionality
 - Adjust RestClient and WebClient builder usage in autoconfiguration
 - Replace direct RestClient.Builder injections with ObjectProvider<RestClient.Builder>
   and WebClient.Builder injections with ObjectProvider<WebClient.Builder>
 - Update ChromaVectorStoreAutoConfiguration to use ObjectProvider
 - Rename MongoDbAtlasLocalContainerConnectionDetailsFactoryTest to IT
 - Switch spring-ai-chroma-store dependency from spring-web to spring-webflux
 - Simplify ChromaApi constructor by using method reference for default headers

 Resolves spring-projects#1066
 Resolves spring-projects#524
@tzolov tzolov force-pushed the webflux-dep-issue branch from ab5308f to 12d3382 Compare October 16, 2024 15:43
@markpollack
Copy link
Member

merged in 2e0a51f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OpenAI Auto Configuration failing to apply Webclient configuration instead of RestClient.
2 participants