support for specifying version numbers in API endpoints #1115
Conversation
…d types in OpenAPI spec
|
I think this one is nearly ready to go (but after #1127). The one change I'm leaning towards is that I think |
|
@jgallagher and @leftwo raised the excellent point that earlier versions of this PR would cause a Dropshot server to happily serve requests for API versions newer than the server was built with. For now, we can make this the responsibility of the Stepping back: my goal for this work is to provide a minimal base for us to go prototype versioning in some real consumers. I think it's pretty likely that as we do that, we will find changes we want to make to this. I wouldn't be surprised if we wind up with some common impls of |
| } | ||
|
|
||
| fn semver_parts(x: &semver::Version) -> (u64, u64, u64) { | ||
| // This was validated during validation. |
| let router = api.into_router(); | ||
| if let VersionPolicy::Unversioned = version_policy { | ||
| if router.has_versioned_routes() { | ||
| return Err(BuildError::UnversionedServerHasVersionedRoutes); |
There was a problem hiding this comment.
is this the point of has_versioned_routes i.e. is it mostly here to produce an error message if the user has done something aberrant?
| api.openapi("Evolving API", semver).write(&mut found).unwrap(); | ||
| let actual = std::str::from_utf8(found.get_ref()).unwrap(); | ||
| expectorate::assert_contents( | ||
| &format!("tests/test_openapi_v{}.json", version), |
There was a problem hiding this comment.
what do you think about
| &format!("tests/test_openapi_v{}.json", version), | |
| &format!("tests/test_versions_v{}.json", version), |
or something to help associate json with rs?
| "semver range (from ... until ...) has the \ | ||
| endpoints out of order", |
There was a problem hiding this comment.
from version is after until version?
| .into_iter() | ||
| .collect(); | ||
| return Err(syn::Error::new_spanned( | ||
| span, |
There was a problem hiding this comment.
I think we want
| span, | |
| input.span(), |
Maybe? In particular I'm thinking of this test:
dropshot/tests/fail/bad_version_backwards.stderr
error: semver range (from ... until ...) has the endpoints out of order
--> tests/fail/bad_version_backwards.rs:17:16
|
17 | versions = "1.2.3".."1.2.2"
|
Should we be underlining both?
| v.value() | ||
| .parse::<semver::Version>() | ||
| .map_err(|e| { | ||
| syn::Error::new_spanned(v, format!("expected semver: {}", e)) |
There was a problem hiding this comment.
what do Semver::parse() errors look like? Are those more useful than something like "{} is not of the form MAJOR.MINOR.PATCH"?
Supersedes #862 (and is based on it). Fixes #869.
Also based on #1127. This includes a test that uses that option to make sure that operations have the same names in different versions, even when they have different handler function names.