Skip to content
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

RFC30: add Serde Decorator to RustClientCodegenPlugin #2650

Open
wants to merge 46 commits into
base: main
Choose a base branch
from

Conversation

thomas-k-cameron
Copy link
Contributor

@thomas-k-cameron thomas-k-cameron commented Apr 27, 2023

Motivation and Context

Add new plugin to RustClientCodegenPlugin

No breaking changes are introduced.

Description

The new plugin adds serde-* features to generated sdk's Cargo.toml.

Users can enable aws-smithy-type's serde features by passing arguments.

Testing

NA

Checklist

NA


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@thomas-k-cameron thomas-k-cameron requested a review from a team as a code owner April 27, 2023 04:23
@thomas-k-cameron thomas-k-cameron mentioned this pull request Apr 27, 2023
2 tasks
@thomas-k-cameron thomas-k-cameron marked this pull request as draft April 27, 2023 05:02
@thomas-k-cameron thomas-k-cameron marked this pull request as ready for review April 27, 2023 06:17
@thomas-k-cameron
Copy link
Contributor Author

NOTE:
Error comes from the lack of feature-gate on published crates.

    Updating crates.io index
error: failed to select a version for `aws-smithy-types`.
    ... required by package `aws-sdk-sso v0.0.0-local (/home/build/workspace/aws-sdk-smoketest/sdk/sso)`
    ... which satisfies path dependency `aws-sdk-sso` of package `aws-config v0.0.0-smithy-rs-head (/home/build/workspace/aws-sdk-smoketest/sdk/aws-config)`
versions that meet the requirements `^0.0.0-smithy-rs-head` are: 0.0.0-smithy-rs-head

the package `aws-sdk-sso` depends on `aws-smithy-types`, with features: `serde-deserialize` but `aws-smithy-types` does not have these features.

@thomas-k-cameron
Copy link
Contributor Author

thomas-k-cameron commented Jun 14, 2023

Oh, it seems to be working now!

override val name: String = "SerdeDecorator"
override val order: Byte = -1

override fun extras(codegenContext: ClientCodegenContext, rustCrate: RustCrate) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to also hook into the ModuleDocSection.ServiceDocs, or even add a new ModuleDocSection.Features, so that these features can be described in the docs. That way, the documentation can inform on the --cfg aws_sdk_unstable requirement. This would require registering a LibRsCustomization, so override fun libRsCustomizations(...).

There's an example here that adds information to the "Crate Organization" documentation section: https://github.com/awslabs/smithy-rs/blob/2f60a5e0904037ec5237c080c61817617b9d8c06/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ClientCustomizations.kt#L19

Copy link
Contributor Author

@thomas-k-cameron thomas-k-cameron Jul 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this function but it seems like there are no changes to the generated code.

Is there anything else that I'm supposed to add?

thomas-k-cameron and others added 4 commits June 29, 2023 17:25
…egen/client/smithy/customize/SerdeDecorator.kt

Co-authored-by: John DiSanti <johndisanti@gmail.com>
@thomas-k-cameron thomas-k-cameron requested a review from a team as a code owner July 4, 2023 02:38
}

// I initially tried to implement with LibRsCustomization but it didn't work some how.
companion object {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the text that is going to be included on the lib.rs docs.
I tried to use the libRsCustomization but somehow it didn't work.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What did it look like when it was overriding libRsCustomization?

Copy link
Contributor Author

@thomas-k-cameron thomas-k-cameron Dec 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was able to make it work!

It looks like this.

image

I tried to put two hash marks,
e.g.

//! # Enabling Unstable Features
//! ## How to enable `Serialize` and `Deserialize`

but I couldn't make it work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to find the commit that didn't work, but it's lost somewhere in the commit history.

@rcoh rcoh requested a review from a team as a code owner November 14, 2023 02:21
@thomas-k-cameron thomas-k-cameron changed the title add Serde Decorator to RustClientCodegenPlugin RFC30: add Serde Decorator to RustClientCodegenPlugin Dec 7, 2023
@thomas-k-cameron thomas-k-cameron requested a review from a team as a code owner December 9, 2023 06:28
@thomas-k-cameron thomas-k-cameron force-pushed the RFC30/serde-decorator branch 2 times, most recently from e659b74 to 44e5fc4 Compare December 10, 2023 12:39
@thomas-k-cameron thomas-k-cameron marked this pull request as draft December 11, 2023 08:15
…codegen/client/smithy/customize/SerdeDecorator.kt
…codegen/client/smithy/customize/SerdeDecorator.kt

modified:   codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/LibRsGenerator.kt
@thomas-k-cameron thomas-k-cameron marked this pull request as ready for review December 11, 2023 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants