Aiming for a single config model

[markmandel]

I attempted to use the model generated by Prost to use for both YAML and xDS -- I got partially there. I got Debug to work, so thought I would try and tackle one of the more complicated Filters - ConcatenateBytes - that one I couldn't make work.

It seems like it might be useful for simple models, but I think it starts to fall apart around when we want defaults for YAML that aren't Option from Prost, and handling Enum's is going to be super tricky.

Context links:

• https://github.com/danburkert/prost/issues/75
• https://serde.rs/remote-derive.html

Thoughts? Useful to keep as an alternate deserialize function for instances where we don't need a second model?

I was also toying with the idea of Serde's remote derive, but that might be too much / doesn't add anything extra.

Or any ideas that I might have missed on how to get the models to align?

Another option is to do something like https://github.com/danburkert/prost#serializing-existing-types - either by hand writing, or copy pasting the models that Prost makes, and then hand annotating them with serde attributes. (the more I think on it, I don't mind this - as you have to write a second model anyway, so why not just copy paste from the code generated by Prost???)

WDYT?

[markmandel]

Another option is to do something like https://github.com/danburkert/prost#serializing-existing-types - either by hand writing, or copy pasting the models that Prost makes, and then hand annotating them with serde attributes. (the more I think on it, I don't mind this - as you have to write a second model anyway, so why not just copy paste from the code generated by Prost???)

Trying this out, and not getting very far:

#[derive(
    Serialize,
    Deserialize,
    Clone,
    Copy,
    Debug,
    PartialEq,
    Eq,
    Hash,
    PartialOrd,
    Ord,
    ::prost::Enumeration,
)]
#[repr(i32)]
pub enum Strategy {
    #[serde(rename = "APPEND")]
    Append = 0,
    #[serde(rename = "PREPEND")]
    Prepend = 1,
}

The rename is still an issue -- tests are still fail because serde is expecting an i32. The only other path I can see it to write a specific serialiser for the enum -- but I don't think that is worth it either. Better to have two models with a conversion function like you have now.

[markmandel]

Ooh, just thought of something useful, lemme paste in the generated models for both Debug and ConcatenateBytes so you can see what they end up looking like:

Debug:

#[derive(serde::Serialize, serde::Deserialize)]
#[serde(rename_all = "camelCase")]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct Config {
    #[prost(message, optional, tag="1")]
    pub id: ::core::option::Option<::prost::alloc::string::String>,
}

ConcatenateBytes:

#[derive(serde::Serialize, serde::Deserialize)]
#[serde(rename_all = "camelCase")]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct Config {
    #[prost(enumeration="Strategy", tag="1")]
    pub strategy: i32,
    #[prost(bytes="vec", tag="2")]
    #[serde(with = "crate::utils::Base64Standard")]
    pub bytes: ::prost::alloc::vec::Vec<u8>,
}
#[derive(serde::Serialize, serde::Deserialize)]
#[serde(rename_all = "camelCase")]
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, ::prost::Enumeration)]
#[repr(i32)]
pub enum Strategy {
    Append = 0,
    Prepend = 1,
}

[iffyio]

This would be nice to have though it seems its not straight-forward and it'll likely require workarounds/limitations which has me thinking its not worth it. I'm not really looking forward to manually updating the generated code to workaround issues either since that'll require a more complex workflow. thinking we might as well keep both structs around, the generated one will only be used once in the codebase so it shouldn't be too noticeable a hassle.

[markmandel]

Yeah, I agree. I'll keep an eye on https://github.com/danburkert/prost/issues/75, which I think ultimately would be the best solution long term. Maybe a long weekend hacking project 😄

Closing this PR then!