Add serde::Serialize/serde::Deserialize to fluent_builders

[thomas-k-cameron]

Describe the feature

It allows you to define/save models with file format supported by serde.

Once this feature is implemented you can,

• save your input to disk in your favourite file format
• load your input from disk using serde

Use Case

It allows you to define/save models with file format supported by serde.

Once this feature is implemented you can,

• save your input to disk in your favourite file format
• load your input from disk using serde

Proposed Solution

add new function fn with_builder() to any structs that implements async fn send().

pub struct CopyObject {
    handle: std::sync::Arc<super::Handle>,
    inner: crate::input::copy_object_input::Builder,
}
impl CopyObject {
    pub(crate) fn with_builder(handle: std::sync::Arc<super::Handle>, inner: crate::input::copy_object_input::Builder) -> Self {
        Self {
            handle,
            inner
        }
    }
}

mod copy_object_input {
    #[derive(std::clone::Clone, std::fmt::Debug, serde::Serialize, serde::Deserialize)]
    struct Builder {
        ...
    }
}

let s3_client = aws_sdk_s3::Client::from(config).copy_object();
let copy_object: CopyObject = s3_client
    .copy_object()
    .with_builder(serde_json::from_str(std::fs::read_to_string("copy_object.json").unwrap()).unwrap())

copy_object.send().await;

Other Information

Potential concerns

1. Binary size may become bigger
2. Compilation time may increase
3. You might want to add #[cfg(feature = "serde")] or something to address the concerns that mentioned above.

Acknowledgements

• I may be able to implement this feature request
• This feature might incur a breaking change

A note for the community

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
• If you are interested in working on this issue, please leave a comment

[Velfi]

@thomas-k-cameron Thanks for submitting this feature request.

related: #269

[thomas-k-cameron]

@Velfi
I read through the #269.
If I understand it right, the reason the serde traits aren't implemented is because of the concerns listed here.
https://awslabs.github.io/smithy-rs/design/faq.html#why-dont-the-sdk-service-crates-implement-serdeserialize-or-serdedeserialize-for-any-types

I don't think my proposal affect the project in the way that your concerned, should I just go ahead in implement it?

[Velfi]

@thomas-k-cameron The rest of the team has been out this week, we'll discuss it today.

My current thoughts on this are that:

• It would require a feature gate
• It would increase the size of the fluent builders module (not an unreasonable cost)
• Would possibly open the way for us to write generic test descriptions that we could share across SDKs. Every once in a while, people talk about sharing tests between the SDKs and having some way to construct inputs would have to be part of that.
• This may require an RFC.
• As part of the PR, I think that we should test compile times with and without the feature to see what kind of cost it has.

Sit tight for now, I don't want you to do any work that we might decide not to incorporate. I'll get back to you today or tomorrow.

[Velfi]

I spoke with the team and we're OK with you implementing this so long as you're OK with some caveats:

• We have concerns about maintaining this feature. For example, what happens if a future update doesn't work with models serialized with a previous version due to things like services adding new fields.
• This entails making service-specific models (like enums) serializable. I have a nagging doubt that that may be difficult or impossible in some situations without creating a manual serialization impl, which isn't scalable.

Given those concerns, we'd only consider merging a feature like this if we could make it clear to users of the feature that it's unstable by gating it behind something like unstable-serde-builders. "Unstable" in this case means it's subject to change or removal at any time.

Also, we'd really appreciate an RFC that describes what user needs this feature provides for, how users will use it, and how it would be implemented in codegen. It would also be nice to see some words on how futures updates to models would affect things. You can see our previous RFCs here.

Let me know if you have any questions or if anything is unclear.

[thomas-k-cameron]

Thank you for the update!

Your RFC template seems to be gone (it gives me 404.).
https://awslabs.github.io/smithy-rs/design/rfcs/rfc_template.html
Do you still have a template?

Also, where should I submit the RFC when it's done?

Replies to the concerns of your team;

• We have concerns about maintaining this feature. For example, what happens if a future update doesn't work with models serialized with a previous version due to things like services adding new fields.

Few idea

1. add #[serde(skip)] to some fields
   or
2. create a different struct which implements serde traits and Into<CorrespondingBuilder>.
   For example, if we are talking about CopyObject then,

// one that we have right now.
mod copy_object_input {
    #[derive(std::clone::Clone, std::fmt::Debug)]
    struct Builder {
        ...
    }
}

mod copy_object_serde {
    #[derive(std::clone::Clone, std::fmt::Debug, serde::Serialize, serde::Deserialize)]
    struct WithSerde {
        ...
    }

    impl Into<copy_object_input::Builder> for WithSerde {
       fn into(copy_object_input::Builder: builder) -> Self {
          ... pieces of code that maps fields of WithSerde to Builder struct
       }
    }
}

In this case, the struct that implements serde traits are going to be completely isolated from the rest of the datatypes, we could even move the structs with serde traits to another crate for a higher level of isolation.

• This entails making service-specific models (like enums) serializable. I have a nagging doubt that that may be difficult or impossible in some situations without creating a manual serialization impl, which isn't scalable.

Can you give me an example?

[jdisanti]

The RFC template can be found here: https://github.com/awslabs/smithy-rs/blob/main/design/src/rfcs/rfc_template.md

RFCs get added to that same directory: https://github.com/awslabs/smithy-rs/blob/main/design/src/rfcs

[thomas-k-cameron]

@jdisanti
thank you very much!

[thomas-k-cameron]

@Velfi @jdisanti
I finished writing the RFC.
Am I supposed to create a PR on Smithy-rs?

https://github.com/thomas-k-cameron/smithy-rs/blob/serialization_and_deserialization/design/src/rfcs/rfc0023_serialization_and_deserialization.md

[jdisanti]

Awesome! Yeah, please do. A PR is the perfect place to discuss and merge it. Thanks!

[thomas-k-cameron]

I just created the PR!
smithy-lang/smithy-rs#1944
Please check it out.

@jdisanti
Thank you for the advice.

[schell]

What's the status on this effort? I have a project that could greatly benefit from this work (it's a terraform-like in pure Rust and being able to serialize wire types would greatly cut down on the amount of boilerplate I have to write).

[thomas-k-cameron]

@schell

We have an active PR available on smithy-lang/smithy-rs#2615 .

I haven't been able to work on it for the past few month, but I should be able to come back to it once I have more free time. I have been planning to start working on it again next week.

FYI: there has been some issues with the unit test. Apparently, some test cases wasn't reading the environment variables properly and I couldn't get it fixed the last time that I tried.

[akinnane]

Is there any time frame for when this work would be finished? Being able to serde::Deserialize API responses without having to implement a intermediate From would be really useful for me. The old SDK had this functionality and implementing Serde for structs is a Interoperability recommendation from the Rust API guidelines. it seems really strange that this is not a higher priority feature for the official rust SDK.

https://rust-lang.github.io/api-guidelines/interoperability.html#c-serde