Closed Bug 1025792 Opened 11 years ago Closed 10 years ago

Handle versioning of the API

Categories

(Hello (Loop) :: Server, defect)

Product:
Hello (Loop)
Component:
Server
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: alexis+bugs, Assigned: mostlygeek)

References

Details

(Whiteboard: [qa+])

Attachments

(1 file)

Link to github PR
56 bytes, text/x-github-pull-request
rhubscher
: review+
Details | Review
The currently deployed loop server doesn't handles API versionning. We should decide on a way to solve this. At a quick glance, it seems easier to debug and to use to have a different endpoint for each version of the service, e.g. loop.services.mozilla.com/v1 (or we could use a subdomain). Since all we do for now on the server has an expiration of one month, it makes sense to not migrate existing data from v0.1 (current, MLP) to v1.0 (MVP). ccing Tarek and Ryan, as they might have some interesting information to add as how we handled versionning for other services.
My understanding is that we can configure NGINX to server MLP version on loop.services.mozilla.com/v1/ and MVP version on loop.services.mozilla.com/v2 Tarek was suggesting that the server could handle both version (if it wasn't too much overhead). In that case, adding a HEADER with the API version can be a good idea, also it makes it easier to debug if we can read the version number directly in the URL. Maybe we can have the loop-server to handle API version using the HEADER version and configure NGINX to configure this header value with regards to the URL prefix?
> Tarek was suggesting that the server could handle both version (if it wasn't too much overhead). yeah the idea is that if we can have the same server handling several protocol versions, it makes it much easier to deploy and support older client without having to keep two different servers running. Adding Benson in the loop so he can give us his feedback on this.
Flags: needinfo?(bwong)
I'm not sure to understand what your proposal is with the header. Are you suggesting we use it or we don't? I don't understand what's the benefit of having one server handling multiple versions of the protocol? While that makes sense if we need backwards compatibility, it doesn't if we don't (and in the case of loop-server, I believe we don't need that).
> While that makes sense if we need backwards compatibility, it doesn't if we don't (and in the case of loop-server, I believe we don't need that). That was the initial point: when you are going to deploy a new set of APIs on the server, will all the clients out there will all work instantly ? or will we need to update them ? if absolutely everything is on the server side with no cache side-effect, then yay! we don't even need to care about the update process. Although, versionning the API is something we should do in any case, if there are other clients developed against our API - and for history/documentation purpose.
The clients will not work with the new APIs, no. But there is no reason we cannot let the old APIs live, and turn them off after a certain period of time. I was actually thinking about the data that remains on the server: we don't need to have the v2 support data from v1. Nevertheless, we'll need to handle old clients, that's correct. I don't exactly understand how we can have two different versions of the API running on the same server, and what that will buy us, especially if the data storage is different.
Do we need to version the whole API, or just messages on the API? (so that fields can be added/remove/changed etc). (In reply to Alexis Metaireau (:alexis) from comment #5) > I was actually thinking about the data that remains on the server: we don't > need to have the v2 support data from v1. Nevertheless, we'll need to handle > old clients, that's correct. Assuming that data === urls mapped to session tokens: Whilst we can probably get away with that for MLP -> MVP, we're not going to be able to long term. If I generate a url today with v1, and then my client updates to v2 tomorrow, I won't be able to access those urls, hence causing folks not to be able to contact me.
> The clients will not work with the new APIs, no. But there is no reason we cannot let the old APIs live, and turn them off after a certain period of time. Sure but that adds maintenance burden. If the new version of the API is just a few changes from the older version, that makes the deployment and maintenance stories simpler. That what we did for some versions of Sync. > I was actually thinking about the data that remains on the server: we don't need to have the v2 support data from v1. Nevertheless, we'll need to handle old clients, that's correct. We should start to document the differences, so we can decide the best strategy. > I don't exactly understand how we can have two different versions of the API running on the same server, ... 99.99% of common code... if version == '1': return Response('v1 style') else: return Response('v2 style') > the data storage is different I propose we start a wiki page for this.
This topic needs more discussion but in general it is better to keep all API version logic in the application and outside of nginx. The app should work even without nginx (or anything else) between it and the client.
Flags: needinfo?(bwong)
Whiteboard: [qa?]
:mostlygeek Can you please explain the rationale behind comment 8?
Flags: needinfo?(bwong)
:dmose we don't want to tightly couple nginx (or anything else) for the app to work. It'll also mean we decouple ops and dev which means more agility for both teams.
Flags: needinfo?(bwong)
:mostlygeek correct me if I am wrong, but this is standard best practice for all our services?
Blocks: 1032741
No longer blocks: 1032741
:mostlygeek, I don't agree with you. I think a server version should handle only one version of the API, the one that it implements and it documents. The nginx should serve whatever server version on whatever path. If we need both v1 and v2, we should deploy loop-server v1 and loop-server v2 and let nginx handle the routing. As soon as v1 is no more hit or supported we can just drop it. For me this keeps the server as clean as possible. Also if you want to run the server without nginx you know exactly which version you want to run so you don't need both and you can set up the path accordingly.
We talked about that with Tarek and Alexis this afternoon and we could have a third possibilty that could leverage our two point of views. If the client could add two headers: Loop-Client-Version: 1.2.0 Loop-Client-Channel: Nightly On every calls, we could redirect to the right API version wrt the client version directly and even handle it directly inside the application as for the simple_push_url to simplePushURL modification. This could also be useful for other tweeks as defined here: https://bugzilla.mozilla.org/show_bug.cgi?id=1033575#c8
(In reply to Rémy Hubscher (:natim) from comment #13) > We talked about that with Tarek and Alexis this afternoon and we could have > a third possibilty that could leverage our two point of views. > > If the client could add two headers: > > Loop-Client-Version: 1.2.0 > Loop-Client-Channel: Nightly > > On every calls, we could redirect to the right API version wrt the client > version directly and even handle it directly inside the application as for > the simple_push_url to simplePushURL modification. > > This could also be useful for other tweeks as defined here: > https://bugzilla.mozilla.org/show_bug.cgi?id=1033575#c8 The scheme we had discussed earlier in the project was: 1) Interfaces would attempt to be forwards-compatible (i.e., they would ignore unknown parameters) 2) Any backwards-compatible changes to an API would not be considered a version change 3) Any non-backwards-compatible changes to an API would change the API endpoint (URL) The problem with having a more rigid versioning structure, rather than a softer set of changes that try to maintain compatibility, is that the more rigid structure requires completely different sets of API handling for each client version actually in use. So we're either supporting a lot of server configurations, or we're causing older clients to stop working unnecessarily.
I think what Adam is proposing is simple and does the trick. The problem with headers is that they're a bit harder to debug than having the version in the url (when you look at the logs for instance, you don't see anything about the headers). Also, versionning in the url is something being used for all the other projects we have at the moment. Now, we need to have clients stick with a version whenever possible. e.g. the url for firefox release could be pointing to v0.10.0 when the url of firefox nightly points to v1.0.0. What we may need here is a sort of discovery mechanism: the clients tells which versions it is able to handle and then the server gives it an endpoint back. This let us deal with which versions are compatible and which one aren't. I know that the tokenserver does something similar to that, so we might use something similar.
(In reply to Alexis Metaireau (:alexis) from comment #15) > (when you look at the logs for instance, you don't see anything about the headers). we can tweak Nginx and Node.js to have that information if needed, but I agree version within the URL are straightforward at all levels. > Also, versionning in the url is something being used for all the other > projects we have at the moment. > > Now, we need to have clients stick with a version whenever possible. e.g. > the url for firefox release could be pointing to v0.10.0 when the url of > firefox nightly points to v1.0.0. > > What we may need here is a sort of discovery mechanism: the clients tells > which versions it is able to handle and then the server gives it an endpoint > back. This let us deal with which versions are compatible and which one > aren't. > > I know that the tokenserver does something similar to that, so we might use > something similar. Yeah good idea, if we are using the URL for the API version, we could rely on basic HTTP codes, e.g. - a 302 that redirects to a compatible API version, with the Location value sent back - a 404 if the API version is not supported anymore
Mozilla Cloud Services needs to have a consistent way of doing versioning across all our (new) service APIs. We had this discussion for the FxA APIs: https://mail.mozilla.org/pipermail/sync-dev/2013-September/000486.html We decided to number API versions 1, 2, 3... and put the version in the URL in the following manner: i.e., https://host/v1/endpoint?key=value... (See: https://github.com/mozilla/fxa-auth-server/blob/master/docs/api.md#url-structure) I recommend the the Loop server API follow this trend. I also consider the call URLs as part of the server API, and I would also recommend versioning them in this manner as well. In terms of API version errors, we define a couple: 1) status code 410, errno 116: endpoint is no longer supported It's gone. 2) status code 400, errno 119: incorrect API version for this account The endpoint is still supported, but you can't use it with this user account. This was support the situation when another of the user's clients had moved on to newer API version in way that made it impossible support concurrent legacy clients. For more detail, see: https://github.com/mozilla/fxa-auth-server/blob/master/docs/api.md#response-format. I understand the pain you guys are experiencing with developing new client software against a new server API. We also experienced it during the push to integrate FxA with Sync in Fx29. We need to try our best to not break people's dev flows, and over-communicate potentially break changes. Regardless, mistakes will happen, and people will get pissed, but it's mostly water under bridge once you release. One suggestion I have is to avoid creating too much legacy solely to cope with the craziness of this initial dev cycle. More than a few times, we set old Nightly clients on fire with API changes during our FxA push. It sucked, but sometimes the cure is worse than the disease.
> Loop-Client-Channel: Nightly What's the use case for this header? It seems unnecessary to me with a versioned API.
(In reply to Chris Karlof [:ckarlof] from comment #18) > I recommend the the Loop server API follow this trend. I also consider the > call URLs as part of the server API, and I would also recommend versioning > them in this manner as well. In terms of the inclusion of the version in the URI string for the *rest* of the API, I'm okay painting this bikeshed the same color as FxA. On the other hand, once we get to the call URLs -- these are user-visible, and passed around. The marketing guys actually looked at TLD registration policies when considering names, trying to see whether they could fit these into "as short as possible" vanity domains. It seems that adding in additional path components kind of defeats some of the goals that the marketing guys had in mind here. (I'll also note that they have somewhat of a different behavior, as they're minted, passed around, and have a life outside of any particular client). For these reasons, I'm not sure a version-based path component *for* *these* *URLs* is necessarily a good idea here.
> For these reasons, I'm not sure a version-based path component *for* *these* *URLs* is necessarily a good idea here. I can buy that. In the absence of other restrictions, the benefits of a verbose, explicit version in the URL is to make it clear what the intention is, regardless of the consumer (client or server). There are different options to version for such situations other than a path component, including the lack of versioning until you need it (as long as legacy systems are built to handle the addition of a version later).
> What we may need here is a sort of discovery mechanism: the clients tells which versions it is able to handle and then the server gives it an endpoint back. This let us deal with which versions are compatible and which one aren't. I'd avoid the complexity unless you need it. What's the use case for discovery? I'm assuming the server always can speak the latest version, as well as some number of legacy versions, and clients will speak whatever version it does. Is this to help facilitate some sort of direct communication between clients, or for some federated model of servers? Again, I'd focus on planned use cases, and defer otherwise.
The discovery mechanism was a way to not hardcode a version in the clients, and let the version be determined at the beginning of the exchange. This was more to open a discussion that an actual solution proposal. It seems that having a simple versioning for the APIs (just containing the "major" makes a lot of sense, since that's the only thing the clients care about (does this new stuff broke my client?). We increase the version number only when non-backward compatible changes are added to the API. In this case, a mail will be sent to dev-media to explain what this does and what's the new API version the client needs to talk. I like how this enforces communication across teams, and that makes it clear that there were an API change. Also, it will force us to keep these as infrequent as possible. The thing we have left to define are: - How long do we maintain old versions of the APIs? - When do we tag the v1 of the API, and should we redirect old clients to v1 automatically?
> The thing we have left to define are: > - How long do we maintain old versions of the APIs? We don't have to decide that now. > - When do we tag the v1 of the API, and should we redirect old clients to v1 automatically? I'd want to release with v1, so I'd do it as soon as possible to make sure all your clients are using it. If things are crazy enough now that you feel you need some versioning in the meantime, you can do some v0.x scheme while in still in dev. This introduces complexity to maintain all these for questionable LT benefit. When you switch to v1 (major version only), you'll probably want to set all the v0.x APIs on fire and redirect those to v1. We had clients use v1 from the start, and just lived through the breakage. For clients that don't include any version, sure, you can redirect automatically.
Okay. In my mind, this versionning / routing should be handled by a reverse proxy (e.g. before hitting the node server), especially because we do versionning only when the API breaks in incompatible ways. I think the next thing we should do is: - Tag a v1.0 for loop; - Be sure ops (Benson?) put the routing in place in the reverse proxy (not sure if that should be NGINX or Zeus); - Be sure to redirect un-specified versions to 1.0; - Notify the clients team they should update the URL they're using (to include the version number). I'm putting a needinfo and assigning to you Benson so you can discuss a bit further on this and tell us if you can do this and ping when it's done.
Assignee: nobody → bwong
Flags: needinfo?(bwong)
API version logic should all be in the application (node.js) code. Pushing routing logic into nginx is too much coupling between proxy/app server. We've done this before and it sucks. I'm a fan of the version in the path approach. Something like https://github.com/expressjs/express-namespace would may make this easier to manage.
Flags: needinfo?(bwong)
> - Be sure ops (Benson?) put the routing in place in the reverse proxy (not sure if that should be NGINX or Zeus); Until we have multiple versions to route, we can defer this issue. FWIW, I agree with Benson that we should probably manage the routing in the application.
Summary: Handle versionning of the API → Handle versioning of the API
Attached file Link to github PR —
Attachment #8475191 - Flags: review?(alexis+bugs)
Comment on attachment 8475191 [details] [review] Link to github PR I'm commenting again here (more info on the github issue). I think we could do things in a clever way. For instance, changing the app methods (get, post, put, delete) so they add directly the prefix to the routes before registering them would mean less changes to the actual code. Also, after having a quick look at the express code, it seems that this could be handled in a completely different way, using routers: http://expressjs.com/api.html#router > A router is an isolated instance of middleware and routes. Routers can be thought of as "mini" applications only capable of performing middleware and routing. Every express application has a builtin app router.
Attachment #8475191 - Flags: review?(alexis+bugs) → review-
Whiteboard: [qa?] → [qa+]
https://github.com/mozilla-services/loop-server/commit/862293718f686735be213fcd681210b9deaff0ea
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
related: redirects should be handled with a 307, not with a 301. see https://github.com/mozilla-services/loop-server/pull/189
Attachment #8475191 - Flags: review- → review+
OK, I can review the commits, but there is a lot here. We should schedule this to go out to Stage and Prod as soon as possible.
Can we get this on the dev server first, so that we can do a quick smoketest with the clients?
We are working on a patch to make sure this doesn't break anything. https://github.com/mozilla-services/loop-server/pull/194/files
Version 0.11.0 is out in Production.
Status: RESOLVED → VERIFIED
Blocks: 1096229
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: