Skip to content
This repository was archived by the owner on Aug 10, 2026. It is now read-only.

Versioning policy - #845

Merged
tbedford merged 6 commits into
mainfrom
versioning
Jan 4, 2021
Merged

Versioning policy#845
tbedford merged 6 commits into
mainfrom
versioning

Conversation

@mattheworiordan

Copy link
Copy Markdown
Member

Given we have no formal policy on how we version, I thought we should get something down based on what we do practically now.

This is not meant to be a discussion about what we should do. That's for another day. This PR is about providing some guidance on what we do now.

@QuintinWillison notice the 1.0 to 1.1 update commit. You may need to do the same for the 1.2 release.

This could be a helper, however some of these references refer to specific library versions. Something to revisit in due course.
@mattheworiordan
mattheworiordan temporarily deployed to ably-docs-staging-pr-845 March 24, 2020 19:52 Inactive
@mattheworiordan

Copy link
Copy Markdown
MemberAuthor

@tomczoink mind looking at why this deployment failed?

index: 19
---

Ably aims to follow the "semantic versioning scheme":http://semver.org/ for our "client library SDKs":https://www.ably.io/download, the "realtime protocol used by these SDKs":/client-lib-development-guide/protocol, our "REST API endpoints":/rest-api, and all other public endpoints, that support versioning, such as "Server-Sent Events":/sse.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We (aim to) follow semver for public-facing APIs (which includes the REST api), but we don't really for the realtime protocol. E.g. connectionTimeserials will be a breaking protocol change, but we're doing that change in 1.3 rather than 2.0 because there's no change to the customer-facing client lib API. The policy we're effectively currently following for the realtime protocol is that breaking changes require a new spec version, which can be either a major or minor version.


Further, given that the version numbers we use cover the raw HTTP API, the protocol and API spec, and the client library SDKs, features such as support for connection/request params, channel params, extras, token lengths, APIs (eg push HTTP API) vary by the API version. Stateless connections are subject to some of those constraints, even if they don't follow the full protocol spec or the library API.

As such, in the case of SSE for example, we feel standardising on versioning means it's quite natural for a connection string for a stateless connection to include the spec version, and that will allow the system to know what features the client can be expected to understand, and might be required so that the system knows how to interpret param values that are supplied by the client. Whilst we always try to make it so that there are no incompatibilities, we have a version because we recognise that that's not always avoidable.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH I'm not sure I'm 100% convinced - the main api version is essentially 100% driven by client library protocol requirements, so this policy leaves other transports wanting to make changes that don't affect client libs in the position of just waiting until the next new client lib spec version happens, which might be a month or > a year away. (And technically per semver if it's a breaking change, waiting until the next new major client lib spec version happens, which is once in a blue moon).

But I don't feel particularly strongly about it, and if you (Matt) and Paddy both think this is the right policy, I'm happy to accept the consensus.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it's not ideal, but equally maintaining different versions across different protocols, SDKs, endpoints feels like a far bigger issue. If we had to introduce a breaking change that only affected SSE for example, and we bumped from 1.3 to 1.4 but communicate it in the changelogs, it would be less than ideal, but possible. Equally, I think we can deal with that issue if it ever arises.

@QuintinWillison

Copy link
Copy Markdown
Contributor

@paddybyers if you're happy then I think we get this merged.

@mattheworiordan
mattheworiordan temporarily deployed to ably-docs-staging-pr-845 March 28, 2020 19:00 Inactive
@mattheworiordan

Copy link
Copy Markdown
MemberAuthor

@paddybyers@SimonWoolf@QuintinWillison updated with a small change. You OK to proceed?

h3(#versioning). Versioning

By default, all requests receive the latest version of the API, which is currently @1.0@.
By default, all requests receive the latest version of the API, which is currently @1.1@.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to modify this file as it's in a versions folder?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good spot, no.

@QuintinWillisonQuintinWillison mentioned this pull request Mar 31, 2020

@paddybyerspaddybyers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, after some clarifications and minor edits (11013bb)

@MarkWoulfeAbly
MarkWoulfeAbly self-requested a review June 12, 2020 10:38

@MarkWoulfeAblyMarkWoulfeAbly left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mattheworiordan if this is still something we want to merge can you please address the conflicts and also update the base version from 1.1 1.2 now we have that released?

@QuintinWillison

QuintinWillison commented Jul 1, 2020

Copy link
Copy Markdown
Contributor

@mattheworiordan are you able to work on this or do you need somebody else to take over?

Also, @paddybyers and @SimonWoolf, I'm here today because I've been briefly diverted to looking at (potentially rogue) 1.x version numbers in client libraries. Most of our libraries released as 1.2.x still have 1.x version numbers for (what I believe) are protocol version numbers:

And then there's the exception (!?) as recently updated by @marto83:

This feels like it needs refinement for clarity (and I've not even looked at the other libraries). In fact, in the case of Java, I think this line is a bug (i.e. idempotent publishing cannot possibly be enabled!). I've created ably/ably-java#590 to track that.

All I can find in the client library features specification is G4 which refers to "API Version" which, clearly, seems to be about public facing programmable APIs - as distinct from information announced to the server at wire level.

I'm not sure that the Realtime Protocol Definition page makes things any clearer either.

What should I be looking at to ascertain what each library should be using? (that is assuming these values are even sent by each library as I've not yet dug deep enough to look at that yet)

@SimonWoolf

SimonWoolf commented Jul 1, 2020

Copy link
Copy Markdown
Member

@QuintinWillison You've correctly identified G4 as the relevant spec item. When it says "A client library must identify to Ably the version..." that is talking about the v= or x-ably-version param that's set by eg Defaults.apiVersion in ably-js. (RTN2f also mentions that "API version param v should be the API version per G4").

So, yeah, all those libraries should have been updated to change that api version with the release of 1.2, and Martin was the only person who remembered :/. (Not your fault obviously, mine as I should have caught it in the 1.2 release PR review)

(Luckily doesn't matter that much, since I don't think there are any non-backwards-compatible wire protocol changes in 1.2. For 1.3 it'll matter a lot more, since the api version will trigger the server to use connectionTimeserials)

@QuintinWillison

Copy link
Copy Markdown
Contributor

@MarkWoulfeAbly I'm going to get this ready to land, FYI.

@kennethkalmer
kennethkalmer changed the base branch from master to mainSeptember 24, 2020 08:34
@QuintinWillisonQuintinWillison removed their assignment Dec 7, 2020
@kavalerov

Copy link
Copy Markdown
Contributor

Quick notes from a call between me, @niksilver , @paddybyers and @QuintinWillison

@kavalerov

Copy link
Copy Markdown
Contributor

@jamienewcomb for some reason this issue was not synced to Jira - can you please take a look?

@jamienewcomb

Copy link
Copy Markdown
Member

We don’t sync PRs :)

@tbedford

Copy link
Copy Markdown
Contributor

I've reviewed locally and looks good, but I'd like to review the deployed review app: it's showing as "1 inactive (outdated) deployment". Is there a way to "reboot" the review app so I can see the latest changes? Unfortunately local review is not enough because I want to check menus are OK.

Comment threadcontent/root/quick-start-guide.textile
@kennethkalmer
kennethkalmer temporarily deployed to ably-docs-versioning-ca8dyfe7y January 4, 2021 11:28 Inactive
@tbedford

Copy link
Copy Markdown
Contributor

@tbedford
tbedford merged commit 5a93a28 into mainJan 4, 2021
@tbedford
tbedford deleted the versioning branch January 4, 2021 12:57
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

10 participants

@mattheworiordan@QuintinWillison@SimonWoolf@kavalerov@jamienewcomb@tbedford@fliptopbox@paddybyers@MarkWoulfeAbly@kennethkalmer