search

oxidecomputer / dropshot

expose REST APIs from a Rust program
Apache License 2.0
865 stars 76 forks source link

dropshot has implicit dependency on schemars and serde versions #67

Open davepacheco opened 3 years ago

davepacheco commented 3 years ago

Dropshot expects some of the consumer's types to impl schemars::JsonSchema, serde::Serialize, and serde::Deserialize. Today, I think consumers add schemars and serde to their Cargo.toml with the same versions as Dropshot and this works fine. However, it's not obvious to consumers that you'd need to do this, or what version(s) would be acceptable here.

I'm trying to better understand the best practice in Rust around this. I expect we'll either want to export JsonSchema from Dropshot and have consumers use that (so they're always using a compatible version, and it's clear where it comes from and what version it is) or else document the dependencies. I'm not sure about serde's interfaces. It seems like overkill to export these, but it also seems like the more correct approach here.

This came up because JsonSchema updated to 0.8. While we updated Dropshot in the master branch (see #61), we didn't bother publishing this to crates.io. So if you grab that version and try to use it with the latest JsonSchema, you get an error about your type not implementing JsonSchema.

ahl commented 3 years ago

For JsonSchema I think we should both export it and maybe wrap up the derive with a macro of our own. I'm 90% we'll want to swap out schemars at some point so better to use our own macro that uses it internally.

Not sure what we should do with regard to serde but happy to have this issue assigned to me either way.

ahl commented 3 years ago

It looks like it may not be possible to re-export serde::{Deserialize,Serialize}: https://github.com/serde-rs/serde/issues/1465

ahl commented 3 years ago

This turns out to be unavoidable for the reasons cited in #70. As @davepacheco suggested in the description: let's just document these dependencies and versions.

jtroo commented 3 years ago

There is a workaround that's found in this comment and is also made use of in the Rocket examples.

The rocket example:

#[derive(Debug, Clone, Deserialize, Serialize)]
#[serde(crate = "rocket::serde")]
struct Post {
    ...
ahl commented 3 years ago

@jtroo thanks for pointing that out. It looks a little onerous and error-prone for consumers to need to specify that extra line for each serde derive.