feat: Replace Integer timeout with Duration in Azure OpenAI properties#152
feat: Replace Integer timeout with Duration in Azure OpenAI properties#152jzb1006 wants to merge 2 commits intolangchain4j:mainfrom
Conversation
- Replace Integer timeout (seconds) with java.time.Duration in ChatModelProperties
- Replace Integer timeout (seconds) with java.time.Duration in EmbeddingModelProperties
- Replace Integer timeout (seconds) with java.time.Duration in ImageModelProperties
- Simplify timeout handling in AutoConfig by removing manual Duration conversion
- Update test configurations to use Duration format (e.g., 60s instead of 60)
Benefits:
- Better type safety with native Duration type
- More intuitive configuration (supports ms, s, m, h units)
- Cleaner code without manual Duration.ofSeconds() conversions
- Aligns with Spring Boot best practices
Configuration examples:
Before: langchain4j.azure-open-ai.chat-model.timeout=60
After: langchain4j.azure-open-ai.chat-model.timeout=60s
langchain4j.azure-open-ai.chat-model.timeout=1m
langchain4j.azure-open-ai.chat-model.timeout=500ms
Resolves TODO comments about using Duration instead of Integer.
dliubarskyi
left a comment
There was a problem hiding this comment.
This seems ot be a breaking change?
This change replaces Integer timeout with Duration while maintaining full backward compatibility for existing configurations. Changes: - Convert property classes from record to class for custom getter logic - Support both legacy Integer (timeout-seconds) and new Duration (timeout) formats - New Duration format takes precedence when both are configured - Add comprehensive backward compatibility tests Benefits: - Zero disruption for existing users (backward compatible) - Improved API for new users (60s, 1m, 500ms formats) - Type safety and no ambiguity about time units - Consistent with Spring Boot ecosystem Test Coverage: - 17 tests, all passing ✅ - Tests for various Duration formats (ms, s, m) - Bean creation verification with both formats - Demonstrates actual usage of new configuration formats Migration Path: Old (deprecated but working): timeout-seconds: 60 New (recommended): timeout: 60s
4a11e3d to
931fd11
Compare
|
@dliubarskyi You're absolutely right to raise this concern! I've updated the PR to provide full backward compatibility while still delivering the benefits of the Duration-based approach. ✅ What's Changed?Before (Breaking Change)
Now (Backward Compatible)
🔧 ImplementationEach property class now supports both formats: // New Duration-based timeout (recommended) // Legacy Integer-based timeout (deprecated, for backward compatibility) public Duration timeout() { Existing users - no action required: Added comprehensive tests (17 tests, all passing ✅):
|
Benefits:
Configuration examples:
Before: langchain4j.azure-open-ai.chat-model.timeout=60 After: langchain4j.azure-open-ai.chat-model.timeout=60s langchain4j.azure-open-ai.chat-model.timeout=1m langchain4j.azure-open-ai.chat-model.timeout=500ms
Resolves TODO comments about using Duration instead of Integer.
Issue
Closes #
Change
General checklist
Checklist for adding new Spring Boot starter
pom.xmlorg.springframework.boot.autoconfigure.AutoConfiguration.importsfile in thelangchain4j-{integration}-spring-boot-starter/src/main/resources/META-INF/spring/directory