Rfc30/feature gate for generated crates #2183

thomas-k-cameron · 2023-01-08T08:32:13Z

Motivation and Context

part of RFC30

Description

add serde::Serialize/Deserialize to RuntimeType.kt
add CfgUnstable on DependencyScope
add serde::Serialize/Deserialize trait to generated data types

Testing

NA

Checklist

TODO

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

This reverts commit 1f33a5c.

...main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/CargoTomlGenerator.kt

smithy-lang#2183 (comment)

* commit RFC * commit * Delete rfc0023_serialization_and_deserialization.md * fix formatting * #1944 (comment) * #1944 (comment) * applied grammarly pre-commit * better example wip * FIX * #1944 (comment) #1944 (comment) #1944 (comment) * add examples for output's builer * fixes * fixing unstable feature gate snippet * Update rfc0028_serialization_and_deserialization.md Section is deleted in response to the discussion #1944 (comment) * file name fix * Update rfc0030_serialization_and_deserialization.md #2183 (comment) * Update rfc0030_serialization_and_deserialization.md

...en-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/CargoDependency.kt

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/RuntimeType.kt

…en/core/smithy/RuntimeType.kt Co-authored-by: Zelda Hessler <[email protected]>

thomas-k-cameron · 2023-01-11T06:47:59Z

@Velfi

#1944 (comment)

Your comment mentions extras method.

Create a SerdeDecorator class that extends ClientCodegenDecorator. This class should be declared in its own file, probably in the smithy-rs/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations folder.

When overriding the extras method, you get access to the RustCrate being generated and you can add features to it by calling rustCrate.mergeFeature(Feature("", default = true, listOf())).

The decorator can add all features that you want to define.

But there ClientCodegenDecorator has no extras method.

What am I missing?

Velfi · 2023-01-11T15:40:34Z

What am I missing?

ClientCodegenDecorator extends CoreCodegenDecorator<ClientCodegenContext>
CoreCodegenDecorator is the class that defines an extras method.
You'll want to extend ClientCodegenDecorator and add an override for extras.

If you search for override fun extras(codegenContext: ClientCodegenContext, rustCrate: RustCrate) { you can find plenty of decorators to use as examples. I hope that's enough to get you going, but let me know if it isn't.

thomas-k-cameron · 2023-01-12T12:01:32Z

Thanks it seems to be working now. No idea why to be honest because the compiler was complaining to me that there is no fun extras to override.

thomas-k-cameron · 2023-01-12T14:21:37Z

There are still things that I haven't figured out so I think I need more time.
#2182

* commit RFC * commit * Delete rfc0023_serialization_and_deserialization.md * fix formatting * smithy-lang/smithy-rs#1944 (comment) * smithy-lang/smithy-rs#1944 (comment) * applied grammarly pre-commit * better example wip * FIX * smithy-lang/smithy-rs#1944 (comment) smithy-lang/smithy-rs#1944 (comment) smithy-lang/smithy-rs#1944 (comment) * add examples for output's builer * fixes * fixing unstable feature gate snippet * Update rfc0028_serialization_and_deserialization.md Section is deleted in response to the discussion smithy-lang/smithy-rs#1944 (comment) * file name fix * Update rfc0030_serialization_and_deserialization.md smithy-lang/smithy-rs#2183 (comment) * Update rfc0030_serialization_and_deserialization.md

thomas-k-cameron · 2023-01-24T18:10:11Z

I think it's ready for reviews!

thomas-k-cameron · 2023-01-24T18:35:04Z

Current implementation adds serde traits to ones with event_stream on it's fields.

But, it would be nice if you could check the other things.

thomas-k-cameron · 2023-01-30T23:21:28Z

Ok, so thanks to NPE the piece of code doesn't work for some reason. No idea why.

#2248

I decided to replace it with a string for a time being to move forward with testing other todos ... etc.

thomas-k-cameron · 2023-01-31T07:37:42Z

#2248 (reply in thread)

pre-commit fix

thomas-k-cameron · 2023-02-03T21:11:13Z

Import errors are handled here #2185.

thomas-k-cameron · 2023-02-04T00:47:45Z

I think I messed it up. Let me fix it.

thomas-k-cameron · 2023-02-04T01:20:13Z

@jdisanti
Can you review this one please?

thomas-k-cameron · 2023-02-17T13:00:25Z

@Velfi
Would you kindly review this one please?

Velfi

For the most part, this is good. I only see a few things that need updating and then we can merge it.

...c/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt

Velfi · 2023-02-17T17:47:43Z

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt

+    public fun SerdeSerialize(): Attribute {
+        return Attribute(cfgAttr(all(writable("aws_sdk_unstable"), feature("serde-serialize")), derive(RuntimeType.SerdeSerialize)))
+    }
+    public fun SerdeDeserialize(): Attribute {
+        return Attribute(cfgAttr(all(writable("aws_sdk_unstable"), feature("serde-deserialize")), derive(RuntimeType.SerdeDeserialize)))
+    }


There's no need for these to be functions. Change them into values and put them in the companion object below.

Suggested change

public fun SerdeSerialize(): Attribute {

return Attribute(cfgAttr(all(writable("aws_sdk_unstable"), feature("serde-serialize")), derive(RuntimeType.SerdeSerialize)))

}

public fun SerdeDeserialize(): Attribute {

return Attribute(cfgAttr(all(writable("aws_sdk_unstable"), feature("serde-deserialize")), derive(RuntimeType.SerdeDeserialize)))

}

val SerdeSerialize = Attribute(cfgAttr(all(writable("aws_sdk_unstable"), feature("serde-serialize")), derive(RuntimeType.SerdeSerialize)))

val SerdeDeserialize = Attribute(cfgAttr(all(writable("aws_sdk_unstable"), feature("serde-deserialize")), derive(RuntimeType.SerdeDeserialize)))

This causes NPE too.

Velfi · 2023-02-17T17:48:31Z

...in/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/RenderSerdeAttribute.kt

+        Attribute("").SerdeSerialize().render(writer)
+        Attribute("").SerdeDeserialize().render(writer)


After making the change suggested in my earlier comment, you can update this function like so:

Suggested change

Attribute("").SerdeSerialize().render(writer)

Attribute("").SerdeDeserialize().render(writer)

Attribute.SerdeSerialize.render(writer)

Attribute.SerdeDeserialize.render(writer)

No, this causes NPE.
Here is the discussion.

#2248

…en/core/smithy/generators/BuilderGenerator.kt Co-authored-by: Zelda Hessler <[email protected]>

thomas-k-cameron · 2023-03-04T06:47:53Z

@Velfi

I left some comments.
Your change request would cause NPE.

I don't want to rush you, but I really appreciate if you could take a look at it.

thomas-k-cameron · 2023-03-09T15:49:41Z

@Velfi

Hi,

Would you kindly give me some replies to my comment?

Velfi

@thomas-k-cameron Sorry about the wait on this one. I'll put it in the merge queue

thomas-k-cameron · 2023-03-10T18:02:52Z

@thomas-k-cameron Sorry about the wait on this one. I'll put it in the merge queue

No problem!
I was worried that I might have done something stupid XD

* Rfc30/document (smithy-lang#2196) * impl/refactor documet * impl serde/desr for document * fix * add anchor end * precommit fix * Rfc30/cargo.toml update (smithy-lang#2180) * add serde under aws_sdk_unstable * changelog.next.toml * Apply suggestions from code review Co-authored-by: Julian Antonielli <[email protected]> Co-authored-by: Julian Antonielli <[email protected]> Co-authored-by: Zelda Hessler <[email protected]> * Rfc30/blob (smithy-lang#2181) * rfc30 blob * blob import fix add license header pre commit * change unstable keyword to aws_sdk_unstable * change unstable keyword to aws_sdk_unstable * add test * updater changelog * Update CHANGELOG.next.toml * Update lib.rs remove extra Document declaration from bad merge * remove/unused import Co-authored-by: Zelda Hessler <[email protected]> Co-authored-by: Zelda Hessler <[email protected]> * RFC30/datetime (smithy-lang#2184) * serde support for datetime with testing * add ciborium to cargo.toml * update changelog * refactor impl suggestions from * refacotring better err message * Revert "update changelog" This reverts commit d58abfa. * add/license header * add: license header * remove unnecessary returns Co-authored-by: Zelda Hessler <[email protected]> * fix: changelog entries * fix: some serde stuff broken by Zelda in the merges * Fix unused imports * Implement serde support for `aws_smithy_types::Number` (smithy-lang#2185) * add line (smithy-lang#2311) * RFC30/Set fields to fluent builder (smithy-lang#2310) * fn set_fields * add fn set_fields to fluent builder * better docs * fix * improve document cfg to Attribute class * Rfc30/feature gate for generated crates (smithy-lang#2183) * add CfgUnstable for feature-gate * add features * add feature-gate * strings for feature gate * Revert "strings for feature gate" This reverts commit 1f33a5c. * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/RuntimeType.kt Co-authored-by: Zelda Hessler <[email protected]> * fix dependency thing on cargo * add OutputShape to builder * EnumGenerator * StructureGenerator * UnionGenerator * todo * fixed? * SerdeDecorator * codegen stuff * update * fix * Apply suggestions from code review Co-authored-by: Zelda Hessler <[email protected]> * - refactoring - pre-commit - https://github.com/awslabs/smithy-rs/pull/2183/files#r1080594621 * adds serde-serialize/deserialize * this one causes null pointer exception * interim solution * new push * fix * add Sensitive Warning * add test for CargoTomlGeneratorTest pre-commit fix * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt Co-authored-by: Zelda Hessler <[email protected]> --------- Co-authored-by: Zelda Hessler <[email protected]> * Upgrade the smithy-rs runtime crates version to 0.55.0 * Update changelog * merge * Revert "merge" This reverts commit 6f00505. * squash commit * update: gradle.properties --------- Co-authored-by: Julian Antonielli <[email protected]> Co-authored-by: Zelda Hessler <[email protected]> Co-authored-by: Zelda Hessler <[email protected]> Co-authored-by: John DiSanti <[email protected]> Co-authored-by: AWS SDK Rust Bot <[email protected]>

thomas-k-cameron added 5 commits January 8, 2023 03:49

add CfgUnstable for feature-gate

9b70288

add features

5c794cf

add feature-gate

6403cec

strings for feature gate

1f33a5c

Revert "strings for feature gate"

2fb7178

This reverts commit 1f33a5c.

LukeMathWalker reviewed Jan 9, 2023

View reviewed changes

...main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/CargoTomlGenerator.kt Outdated Show resolved Hide resolved

Merge branch 'main' into rfc30/feature-gate-for-generated-crates

3513d7d

thomas-k-cameron added a commit to thomas-k-cameron/smithy-rs that referenced this pull request Jan 9, 2023

Update rfc0030_serialization_and_deserialization.md

8b7ee9f

smithy-lang#2183 (comment)

thomas-k-cameron mentioned this pull request Jan 9, 2023

UPDATE for RFC30: Serialization and deserialization #2186

Merged

Velfi reviewed Jan 10, 2023

View reviewed changes

...en-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/CargoDependency.kt Show resolved Hide resolved

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/RuntimeType.kt Outdated Show resolved Hide resolved

thomas-k-cameron and others added 6 commits January 11, 2023 09:28

Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codeg…

1717d72

…en/core/smithy/RuntimeType.kt Co-authored-by: Zelda Hessler <[email protected]>

fix dependency thing on cargo

e16ce62

add OutputShape to builder

14d5e1d

EnumGenerator

cc61ede

StructureGenerator

22ef698

UnionGenerator

7aaa744

todo

8b12529

thomas-k-cameron added 2 commits January 12, 2023 11:56

fixed?

cd602bf

SerdeDecorator

b90a1b2

thomas-k-cameron added 4 commits January 12, 2023 12:03

codegen stuff

1cefde2

Merge branch 'main' into rfc30/feature-gate-for-generated-crates

af24d35

update

25ed6ff

fix

6a3757e

thomas-k-cameron changed the base branch from main to unstable-serde-support January 13, 2023 00:29

thomas-k-cameron requested review from jjant and unexge and removed request for a team January 24, 2023 18:10

thomas-k-cameron added 2 commits January 27, 2023 16:21

this one causes null pointer exception

e0811b2

interim solution

a5c2eb8

new push

8ceb679

thomas-k-cameron added 3 commits February 3, 2023 18:48

fix

b3fa786

add Sensitive Warning

9f84174

add test for CargoTomlGeneratorTest

a5970c0

pre-commit fix

thomas-k-cameron requested a review from a team as a code owner February 4, 2023 00:44

thomas-k-cameron force-pushed the rfc30/feature-gate-for-generated-crates branch from c6b0676 to a5970c0 Compare February 4, 2023 00:49

Velfi requested changes Feb 17, 2023

View reviewed changes

Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codeg…

4e8a72c

…en/core/smithy/generators/BuilderGenerator.kt Co-authored-by: Zelda Hessler <[email protected]>

thomas-k-cameron requested review from Velfi and removed request for jjant and unexge March 9, 2023 19:26

Velfi approved these changes Mar 10, 2023

View reviewed changes

Velfi merged commit 52865bb into smithy-lang:unstable-serde-support Mar 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rfc30/feature gate for generated crates #2183

Rfc30/feature gate for generated crates #2183

thomas-k-cameron commented Jan 8, 2023 •

edited

Loading

thomas-k-cameron commented Jan 11, 2023

Velfi commented Jan 11, 2023

thomas-k-cameron commented Jan 12, 2023

thomas-k-cameron commented Jan 12, 2023

thomas-k-cameron commented Jan 24, 2023

thomas-k-cameron commented Jan 24, 2023 •

edited

Loading

thomas-k-cameron commented Jan 30, 2023 •

edited

Loading

thomas-k-cameron commented Jan 31, 2023

thomas-k-cameron commented Feb 3, 2023

thomas-k-cameron commented Feb 4, 2023

thomas-k-cameron commented Feb 4, 2023

thomas-k-cameron commented Feb 17, 2023

Velfi left a comment

Velfi Feb 17, 2023

thomas-k-cameron Feb 19, 2023

Velfi Feb 17, 2023

thomas-k-cameron Feb 18, 2023

thomas-k-cameron commented Mar 4, 2023

thomas-k-cameron commented Mar 9, 2023

Velfi left a comment

thomas-k-cameron commented Mar 10, 2023

		Attribute("").SerdeSerialize().render(writer)
		Attribute("").SerdeDeserialize().render(writer)

Rfc30/feature gate for generated crates #2183

Rfc30/feature gate for generated crates #2183

Conversation

thomas-k-cameron commented Jan 8, 2023 • edited Loading

Motivation and Context

Description

Testing

Checklist

thomas-k-cameron commented Jan 11, 2023

Velfi commented Jan 11, 2023

thomas-k-cameron commented Jan 12, 2023

thomas-k-cameron commented Jan 12, 2023

thomas-k-cameron commented Jan 24, 2023

thomas-k-cameron commented Jan 24, 2023 • edited Loading

thomas-k-cameron commented Jan 30, 2023 • edited Loading

thomas-k-cameron commented Jan 31, 2023

thomas-k-cameron commented Feb 3, 2023

thomas-k-cameron commented Feb 4, 2023

thomas-k-cameron commented Feb 4, 2023

thomas-k-cameron commented Feb 17, 2023

Velfi left a comment

Choose a reason for hiding this comment

Velfi Feb 17, 2023

Choose a reason for hiding this comment

thomas-k-cameron Feb 19, 2023

Choose a reason for hiding this comment

Velfi Feb 17, 2023

Choose a reason for hiding this comment

thomas-k-cameron Feb 18, 2023

Choose a reason for hiding this comment

thomas-k-cameron commented Mar 4, 2023

thomas-k-cameron commented Mar 9, 2023

Velfi left a comment

Choose a reason for hiding this comment

thomas-k-cameron commented Mar 10, 2023

thomas-k-cameron commented Jan 8, 2023 •

edited

Loading

thomas-k-cameron commented Jan 24, 2023 •

edited

Loading

thomas-k-cameron commented Jan 30, 2023 •

edited

Loading