Cloud SDK generator with oneOf fix #186
Conversation
| <enumUnknownDefaultCase>true</enumUnknownDefaultCase> | ||
| <useBeanValidation>false</useBeanValidation> | ||
| <useOneOfInterfaces>true</useOneOfInterfaces> |
There was a problem hiding this comment.
useOneOfInterfaces is the only addition compared to the old Cloud SDK generator
sample-code/spring-app/src/test/java/com/sap/ai/sdk/app/controllers/OrchestrationTest.java
Outdated
Show resolved
Hide resolved
sample-code/spring-app/src/test/java/com/sap/ai/sdk/app/controllers/OrchestrationTest.java
Outdated
Show resolved
Hide resolved
orchestration/src/main/java/com/sap/ai/sdk/orchestration/client/OrchestrationCompletionApi.java
Outdated
Show resolved
Hide resolved
orchestration/src/main/java/com/sap/ai/sdk/orchestration/client/OrchestrationCompletionApi.java
Outdated
Show resolved
Hide resolved
orchestration/src/main/java/com/sap/ai/sdk/orchestration/client/model/AzureThreshold.java
Show resolved
Hide resolved
Pull Request is not mergeable
CharlesDuboisSAP
dismissed
MatKuhr’s stale review
This is https://github.tools.sap/AI/llm-orchestration/blob/v0.29.3/src/spec/api.yaml and not the API Hub
| final ObjectNode requestJson = JACKSON.createObjectNode(); | ||
| requestJson.set("messages_history", JACKSON.valueToTree(prompt.getMessagesHistory())); | ||
| requestJson.set("input_params", JACKSON.valueToTree(prompt.getTemplateParameters())); |
There was a problem hiding this comment.
(Question/Minor)
Why not use CompletionPostRequest class as done before your change?
Your change requires the developer to know the keys are called "messages_history" and "input_params".
There was a problem hiding this comment.
Because Matthias built an invalid object that our builder doesn't allow anymore. Plus the developer still has to know the key orchestration_config (line 184)
There was a problem hiding this comment.
The object itself isn't invalid, it's just not built all in one line of code 😉
We could still use CompletionPostRequest and just set the config to null explicitly, but then we have to suppress the warning, and overall it doesn't look much better. Alternatively, we could just make the constructors public, but I guess we don't want that for the few users who would use the low-level API and should be forced to supply all mandatory parameters..
There is now no diff on the spec |
Context
AI/ai-sdk-java-backlog#112.
oneOf / anyOf / allOf with multiple possible options should form an interface or a class that is able to parse those multiple options
Feature scope:
Definition of Done
Aligned changes with the JavaScript SDKRelease notes updated