-
Notifications
You must be signed in to change notification settings - Fork 31
Service sdk json #1367
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
Service sdk json #1367
Conversation
…hy-kotlin into server-sdk-main # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
…n into service-sdk-serde # Conflicts: # codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinDependency.kt # codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/RuntimeTypes.kt # codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/service/ServiceStubGenerator.kt
…ntime rather than compile
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
) | ||
.write("var response: Any") | ||
.write("var response: #T = $defaultResponse", serializerResultSymbol) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we sending default responses now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In renderSerializeHttpBody
, if the output shape does not have a body, response
will not be set, which causes an error indicating that response
must be initialized.
...lin/software/amazon/smithy/kotlin/codegen/rendering/protocol/HttpBindingProtocolGenerator.kt
Outdated
Show resolved
Hide resolved
KotlinTypes.Text.encodeToByteArray, | ||
) | ||
if (ctx.settings.build.generateServiceProject) { | ||
writer.write("response = input.#L.value.toString()", memberName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be int
since it's an INT_ENUM
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In serialization, JSON protocol will return a string as the response body. So, we need to convert everything in String
tests/codegen/service-codegen-tests/src/test/kotlin/com/test/CborServiceTest.kt
Show resolved
Hide resolved
tests/codegen/service-codegen-tests/src/test/kotlin/com/test/utils.kt
Outdated
Show resolved
Hide resolved
...lin/software/amazon/smithy/kotlin/codegen/rendering/protocol/HttpBindingProtocolGenerator.kt
Outdated
Show resolved
Hide resolved
tests/codegen/service-codegen-tests/model/service-json-test.smithy
Outdated
Show resolved
Hide resolved
Affected ArtifactsChanged in size
|
Issue #
Description of changes
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.