Skip to content

Conversation

zhuyuy
Copy link

@zhuyuy zhuyuy commented Nov 3, 2024

fix #1622

Copy link
Contributor

@jitokim jitokim left a comment

Choose a reason for hiding this comment

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

👍

@markpollack
Copy link
Member

This condition will always be true. In the issue described it looks to be a matter of providing the spring retry library into the project. It is defined in spring-ai-openai module

		<dependency>
			<groupId>org.springframework.ai</groupId>
			<artifactId>spring-ai-retry</artifactId>
			<version>${project.parent.version}</version>
		</dependency>

in short, I don't think PR is solving the problem.

@markpollack markpollack added this to the 1.0.0-M4 milestone Nov 5, 2024
@jitokim
Copy link
Contributor

jitokim commented Nov 5, 2024

In the issue #1622 , spring-ai-azure-openai model is used, not spring-ai-openai.
But it has not spring-ai-retry module.
(it is same in some other modules)

https://github.com/spring-projects/spring-ai/blob/main/models/spring-ai-azure-openai/pom.xml

i think the other solution is adding spring-ai-retry module to starter like spring-ai-starter-azure-openai

https://github.com/spring-projects/spring-ai/blob/v1.0.0-M3/spring-ai-spring-boot-starters/spring-ai-starter-azure-openai/pom.xml

Isn't pr's solution better?

@zhuyuy @markpollack What do you think?

@zhuyuy
Copy link
Author

zhuyuy commented Nov 6, 2024

Yes, the issue uses spring-ai-azure-openai, not spring-ai-openai, it hasn't spring-ai-retry.
Expected @ConditionalOnClass is false, but actually is true, because the condition is org.springframework.retry.support.RetryTemplate, a class in spring-retry, not spring-ai-retry.
I believe that this condition is indeed unreasonable. PR is fine

@markpollack
Copy link
Member

Thanks for the clarification. indeed, for those implementations that don't use Spring Retry (as they already provide it in the SDK or we just haven't gotten to it yet) it would pose a problem. Testing for a fix in spring-ai-retry is the way to go.

@markpollack markpollack self-assigned this Nov 6, 2024
@markpollack markpollack added the bug Something isn't working label Nov 6, 2024
@markpollack
Copy link
Member

merged in 99d9864

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

App fails to boot on Cloud Foundry when using Spring Cloud Services Service Discovery
3 participants