Skip to content

Fix incorrect ollama autoconfiguration changes #3645

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dev-jonghoonpark
Copy link
Contributor

@dev-jonghoonpark dev-jonghoonpark commented Jun 23, 2025

Recently, with the merge of #1852, file from the old AutoConfiguration path were unintentionally restored.

Currently, AutoConfiguration classes are located under /auto-configurations/**.
However, OllamaAutoConfiguration is located in /spring-ai-spring-boot-autoconfigure/src/main/java/org/springframework/ai/autoconfigure/ollama/OllamaAutoConfiguration.java.

To address this, the file has been removed, and the originally intended changes have been added to the OllamaApiAutoConfiguration and OllamaChatAutoConfiguration files.

I have confirmed that the following tests pass successfully:

./mvnw verify -Pintegration-tests -pl auto-configurations/models/spring-ai-autoconfigure-model-ollama
./mvnw verify -Pintegration-tests -pl models/spring-ai-ollama

@markpollack @apappascs


Additionally, I noticed an incorrect method name in the OllamaEmbeddingModelObservationIT test and made the necessary correction:

  • openAiApiollamaApi
  • openAiEmbeddingModelollamaEmbeddingModel

@dev-jonghoonpark dev-jonghoonpark force-pushed the fix-incorrect-ollama-autoconfiguration-changes branch from a0c55ae to 5ee2ed7 Compare June 23, 2025 14:41
@dev-jonghoonpark dev-jonghoonpark marked this pull request as draft June 23, 2025 15:17
@dev-jonghoonpark dev-jonghoonpark force-pushed the fix-incorrect-ollama-autoconfiguration-changes branch from 5ee2ed7 to 0c5d9be Compare June 23, 2025 16:54
@dev-jonghoonpark dev-jonghoonpark force-pushed the fix-incorrect-ollama-autoconfiguration-changes branch from 0c5d9be to e26341f Compare June 23, 2025 23:44
@dev-jonghoonpark dev-jonghoonpark marked this pull request as ready for review June 23, 2025 23:44
@dev-jonghoonpark dev-jonghoonpark marked this pull request as draft June 24, 2025 00:09
@dev-jonghoonpark dev-jonghoonpark force-pushed the fix-incorrect-ollama-autoconfiguration-changes branch from e26341f to 0d9a289 Compare June 24, 2025 01:18
@dev-jonghoonpark dev-jonghoonpark marked this pull request as ready for review June 24, 2025 01:47
Copy link
Contributor

@sunyuhan1998 sunyuhan1998 left a comment

Choose a reason for hiding this comment

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

Is the name of the method openAiEmbeddingModel in OllamaEmbeddingModelObservationIT also incorrect?

public OllamaEmbeddingModel openAiEmbeddingModel(OllamaApi ollamaApi,
TestObservationRegistry observationRegistry) {
return OllamaEmbeddingModel.builder().ollamaApi(ollamaApi).observationRegistry(observationRegistry).build();
}

@dev-jonghoonpark
Copy link
Contributor Author

@sunyuhan1998 Good catch. I'll fix that part as well

@dev-jonghoonpark dev-jonghoonpark force-pushed the fix-incorrect-ollama-autoconfiguration-changes branch from 0d9a289 to fa5e287 Compare June 24, 2025 02:01
@sunyuhan1998
Copy link
Contributor

@sunyuhan1998 Good catch. I'll fix that part as well

Actually, I also found another similar issue, but it's not closely related to the content of this PR :

public MistralAiEmbeddingModel openAiEmbeddingModel(MistralAiApi mistralAiApi,
TestObservationRegistry observationRegistry) {
return new MistralAiEmbeddingModel(mistralAiApi, MetadataMode.EMBED,
MistralAiEmbeddingOptions.builder().build(), RetryTemplate.defaultInstance(), observationRegistry);
}

@dev-jonghoonpark
Copy link
Contributor Author

@sunyuhan1998 👍
It might be better to address the part related to MistralAiEmbeddingModelObservationIT.java in a separate PR.

@apappascs
Copy link
Contributor

@sunyuhan1998 Good catch. I'll fix that part as well

Actually, I also found another similar issue, but it's not closely related to the content of this PR :

public MistralAiEmbeddingModel openAiEmbeddingModel(MistralAiApi mistralAiApi,
TestObservationRegistry observationRegistry) {
return new MistralAiEmbeddingModel(mistralAiApi, MetadataMode.EMBED,
MistralAiEmbeddingOptions.builder().build(), RetryTemplate.defaultInstance(), observationRegistry);
}

fixing that with this pr #3652

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

Successfully merging this pull request may close these issues.

4 participants