[Orchestration] OSS Generator Improvements #158
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
newtork
reviewed
Nov 14, 2024
| JsonParser parser, DeserializationContext context, JavaType concreteType) throws IOException { | ||
| var defaultDeserializer = context.findRootValueDeserializer(concreteType); | ||
| return (LLMModuleResult) defaultDeserializer.deserialize(parser, context); | ||
| return mapper.readValue(rootNode.toString(), LLMModuleResultSynchronous.class); |
There was a problem hiding this comment.
As discussed we'll only consider one implementation for the interface during deserialization.
I'm only wondering whether this can be solved in an easier way than dedicated deserializer.
newtork
reviewed
Nov 14, 2024
Comment on lines
+18
to
+20
| @javax.annotation.Generated( | ||
| value = "org.openapitools.codegen.languages.JavaClientCodegen", | ||
| comments = "Generator version: 7.9.0") |
There was a problem hiding this comment.
(Comment/Minor)
Still wondering whether this can be disabled?
MatKuhr
added a commit
that referenced
this pull request
Nov 14, 2024
* All hail the new orchestration client * Leftovers * Update code generatino ADR * Minor comments * Formatting * Add javadoc and logging * Added. More. Javadoc. * Update sample HTML page * Switch to error for empty content filter configs * Simplify Exception * Fix finish reason * Fix type of Orchestration Response Messages * Minor improvements * Switch to deployment * Update pom for using OSS generator * Using original orchestration spec. Test contain descriminator * Used OpenAPI generator for orchestration * Using discriminator spec everywhere. test success. * LLMModule specific deserializer ready. Test success. * Generalized Fallback deserializer ready. Test success. * OSS generator on customer facing orchestration spec - Introduce a generic fallback and LLMModuleResult specific deserializers. - Generator ignoring all interfaces - Adapt jackson annotations * Merging changes from main * Merging changes from main * Merging changes from main * Formatting * Clean up - Remove Generic fallback deserializer - Beta Annotation on all concrete model classes. - Introduce Mixin to override model annotations - Improve variable naming * Update generated models * Add test and improve deserializer - Make deserializer lenient to allowed values * [Orchestration] OSS Generator Improvements (#158) * Simplifications and cleanup * Remove parsing for streaming * Reduce visibility of mixin class * replace solution with custom deserializer to mixin * remove unnecessary dependency jackson-databind-nullable * Minor newline-reduction * Minor syntax sugar --------- Co-authored-by: Matthias Kuhr <matthias.kuhr@sap.com> Co-authored-by: SAP Cloud SDK Bot <cloudsdk@sap.com> Co-authored-by: Roshin Rajan Panackal <roshin.rajan.panackal@sap.com> Co-authored-by: I538344 <charles.dubois@sap.com> Co-authored-by: Matthias Kuhr <52661546+MatKuhr@users.noreply.github.com> Co-authored-by: Alexander Dümont <alexander_duemont@web.de> Co-authored-by: Alexander Dümont <22489773+newtork@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
-Dgenerateproperty flagAlso contains a merge from main branch