-
Notifications
You must be signed in to change notification settings - Fork 190
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
Codegenerate Python application example and add explicit cast during JSON deserialization #1520
Conversation
… types in JSON deserialization if the shape has been overriden by the symbol provider
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
...are/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonApplicationGenerator.kt
Outdated
Show resolved
Hide resolved
...are/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonApplicationGenerator.kt
Outdated
Show resolved
Hide resolved
A new generated diff is ready to view.
A new doc preview is ready to view. |
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.
Overall, looks good to me. Just a few suggestions on ParserUtil
.
...gen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/ParserUtil.kt
Outdated
Show resolved
Hide resolved
...gen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/ParserUtil.kt
Outdated
Show resolved
Hide resolved
* with wrappers compatibile with Python, without touching the original implementation coming from `aws-smithy-types`. | ||
*/ | ||
class ParserUtil(private val symbolProvider: RustSymbolProvider, private val runtimeConfig: RuntimeConfig) { | ||
fun convertViaFrom(shape: Shape): Writable = |
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.
How this is written now assumes the conversion will always be a .something()
suffix, which may not always be the case. I think this would be a lot more flexible if it took a String or Writable as an argument that represents the value/expression being converted, and then places that in the correct place itself.
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.
I am not sure I understand properly what you are suggesting here...
What do you mean with expect .something()
to always exists?
I thought that passing a shape and adding the suffix to it in case it differs from the original shape was the most general way of achieving this.
Can you expand a little on your proposal?
…ithy/protocols/parse/ParserUtil.kt Co-authored-by: John DiSanti <[email protected]>
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
Motivation
See #1367.
Description
from
complex types in JSON deserialization if the shape has been overriden by the symbol provider.Checklist
CHANGELOG.next.toml
if I made changes to the smithy-rs codegen or runtime cratesCHANGELOG.next.toml
if I made changes to the AWS SDK, generated SDK code, or SDK runtime cratesBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.