Re: [pbs-devel] [RFC PATCH proxmox-backup 1/3] api-types: make UploadStatistic an api type

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Mon, Nov 27, 2023 at 11:17:49AM +0100, Dominik Csapak wrote:
> On 11/27/23 11:12, Thomas Lamprecht wrote:
> > On 27.11.23 11:01, Dominik Csapak wrote:
> > > On 11/27/23 10:52, Thomas Lamprecht wrote:
> > > > On 01.08.23 11:29, Dominik Csapak wrote:
> > > > > +#[api()]
> > > > > +#[derive(Copy, Clone, Serialize, Deserialize)]
> > > > 
> > > > misses:
> > > > 
> > > > #[serde(rename_all = "kebab-case")]
> > > > 
> > > > or does that break manifest?
> > > 
> > > it shouldn't since we only save it in the 'unprotected' field that is a 'Value'
> > > but i'll check
> > 
> > I did not mean breakage as in "might break signatures", but as in
> > backward/forward compat to any of our code/tools using that field
> > already (tbh. not sure from top of my head if serde json magically
> > falls back to field casing variants, e.g., check if foo-bar is there
> > if not take foo_bar, I think not, so that's why I asked - should have
> > said so in my first response).
> 
> mhmm.. we only ever write that once (during backup finish) and never read
> it anywhere (until my patch) so it couldn't break any existing code.
> 
> adding the rename now would only affect new backups, but you might be
> right that deserializing older backups might not be working then.
> 
> again, i'll check

in theory we could also add a #[serde(alias = "...")]. The docs say
"deserialize (...) from the given name *or* from its Rust name"[1], but
I'm not sure if "its Rust name" then means the original or the
`rename_all`-transformed version ;-)
Also, we don't currently directly support this in the proxmox-schema -
but we do in perl iirc, so we might as well add that to rust?

[1] https://serde.rs/field-attrs.html