Expose dropshot's http and hyper dependencies

[jmpesp]

Users constructing dropshot::HttpError manually do not have access to
the StatusCode type, and instead of requiring them to keep their http
dependency in sync with dropshot expose dropshot's StatusCode. That way,
users can do:

HttpError {
    status_code: dropshot::StatusCode::NOT_FOUND,
    ...
}

[ahl]

Good idea! I like it. @davepacheco any objections?

[jmpesp]

I might even propose generalizing this to expose dropshot's http - so one can do something like

dropshot::http::Response::builder()
  .status(dropshot::http::StatusCode::UNAUTHORIZED)
  ....

[jmpesp]

Plus hyper? :)

[davepacheco]

Yeah, I can see how this is annoying for consumers. But would exposing http (and hyper?) cause problems for consumers that already need their own version? I guess they'd have to keep that in sync, but they already have to do that, so maybe it's no worse?

I remember running into problems with a similar idea under #70. But I think in that case the reason is had to do with the crate in question (json-schema) exposing a macro. I don't think we're using http or hyper macros here. I wonder if the same advice is worthwhile: try updating omicron to a version of dropshot with this change before we commit to it here, in case we find some unforeseen problem with how this would affect consumers?

[davepacheco]

(sorry for the double-comment)

I think some crates do use this pattern of publicly exposing some internal dependency. I went to check hyper, which exposes some http items). As a new user, I remember finding this very confusing. While the page is labeled hyper::StatusCode, the docs come from http, and sometimes use terms differently than the module I thought the item was in.

Are there any guidelines on this best practice? I did find this draft. I found a few other discussions. It seems like this is reasonably idiomatic?

[jmpesp]

I can't speak to how idiomatic it is, but I got the idea from tokio-rustls.

I was also searching for a way for cargo to track the dependency of a dependency, but that seems a little more heavy. This way when dropshot updates a dependency there aren't any changes to consumer code.

[davepacheco]

(sorry again -- I should probably finish whatever digging I'm going to do before commenting here :-/)

Interestingly, I'm not the only one that finds that doc behavior confusing. And this comment about whether every crate that exposes future-stuff ought to re-export futures raises a similar question to the one above. Suppose we have multiple crates in omicron that use http::StatusCode. If omicron wants to share a http::StatusCode between them, could it wind up in the bizarre position of having to convert foo::http::StatusCode::NOT_FOUND to dropshot::http::StatusCode::NOT_FOUND? Or maybe we avoid that by saying dropshot and foo should have relaxed semver constraints on http such that Cargo will wind up picking the same one? Is that better than the status quo?

[jmpesp]

If omicron wants to share a http::StatusCode between them, could it wind up in the bizarre position of having to convert foo::http::StatusCode::NOT_FOUND to dropshot::http::StatusCode::NOT_FOUND?

This sentence made me think that it would be better if Cargo could follow a dependency of a dependency. Some fake syntax in omicron like:

http = { transitive = ["dropshot", "foo"] }

which interestingly could enforce that foo and dropshot have matching http versions (does Cargo already complain if the http versions can't be made to match?). But this would complicate the dep graph quite a bit I think.

[ahl]

@davepacheco FWIW I believe the issue with #70 as it pertained to JsonSchema (and serde derive macros) was unique to derive macros in that their generated code embeds particular crate names.

[jmpesp]

Closing this