-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Conversation
e2a0508
to
ab5308f
Compare
@@ -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); |
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'll run the IT to make sure adding content type is ok.
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.
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 -> { |
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.
could be inlined as there is just one usgae.
@@ -34,6 +34,7 @@ | |||
<version>${spring-retry.version}</version> | |||
</dependency> | |||
|
|||
<!-- --> |
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.
can remove extraneous comment.
|
||
return new AnthropicApi(connectionProperties.getBaseUrl(), connectionProperties.getApiKey(), | ||
connectionProperties.getVersion(), restClientBuilder, webClientBuilder, responseErrorHandler, | ||
connectionProperties.getVersion(), restClientBuilderProvider.getIfAvailable(RestClient::builder), |
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.
if unavailable, we should create it.
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.
In contrary we should. This is the purpose of using the ObjectProvider
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.
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
ab5308f
to
12d3382
Compare
merged in 2e0a51f |
Resolves #1066, #524