Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only include the major version in the Tus-Resumable header #96

Open
Acconut opened this issue Sep 20, 2016 · 7 comments
Open

Only include the major version in the Tus-Resumable header #96

Acconut opened this issue Sep 20, 2016 · 7 comments
Milestone

Comments

@Acconut
Copy link
Member

Acconut commented Sep 20, 2016

In an unrelated discussion (#79 (comment)), @ruv left an interesting link (https://www.mnot.net/blog/2012/12/04/api-evolution) which drew my attention towards how we address versioning inside the tus protocol. The current state is that we have the Tus-Version header which indicates which versions of the protocol is supported by the Server and the Tus-Resumable header which indicates which version the Client or the Server used in its request or response. My concern in this proposal is mostly about later one. Currently, the specification defines it as following:

The value MUST be the version of the protocol used by the Client or the Server.

Since v1.0.0 has been the only released version which included the Tus-Resumable header, handling versions has been relatively easy as its value was always 1.0.0. However as we are approaching the 1.1 release, this may introduce a few issues:

As some Clients will be updated to use the new 1.1 version, Server may still only accept the previous 1.0 version. Since the Server is allowed to reject unsupported version, an upload will not be possible. For these scenarios, tus has provided the Tus-Version header which allows the Client to know which versions it may use. In this example, the Client should downgrade to using 1.0, but this negotiation step results in a overhead which could be avoidable.

Furthermore, including the entire version in the Tus-Resumable header is also rather useless. Let me try to explain this point:

  • If we publish a new patch release, which just fixes a misspelled word or phrase, neither Client nor Server care about this, since the actual meaning of the specification must not have changed (according to SemVer).
  • If we publish a new minor release, which adds a new feature, we must make this new feature a new optional extension or else it would be a breaking change and this is not allowed in a minor release. Furthermore, versions can not be used as an indicator for which extensions are supported by the Server, as the are optional. For these situation, we introduced the Tus-Extension header which cares this exact information.
  • If we publish a new major release, we have introduced a backwards-incompatible change and both Client and Server implementation need to be aware of this situation.

What I wanted to say with this enumeration is that the only important piece of information inside the version for the Client and Server, is the major number. Therefore, I propose to change the Tus-Resumable header to only include the major part of the version, such as 1.0.0 or 2.0.0. At the moment, this does not introduce a breaking change as we only released 1.0.0. Alongside with this, the Tus-Version header should only list the major versions supported by the Server.

This also follows the advice Mark Nottingham gave in his blog post (https://www.mnot.net/blog/2012/12/04/api-evolution):

Keep Compatible Changes Out of Names

I recommend you read to it. It's a great piece in my mind.

To summarize the advantages:

  • Eliminate possible negotiation step in most situations
  • Simpler Server and Client implementations due to simpler versioning rules
  • Future-proof implementations which are compatible to newer version without making changes.

In the end, I am looking forward to hearing some feedback (@kvz @bhstahl @vayam).

@Acconut Acconut added this to the 1.1 milestone Sep 20, 2016
@Acconut
Copy link
Member Author

Acconut commented Sep 26, 2016

Just wanted to remind you that any kind of feedback is appreciated.

@bhstahl
Copy link

bhstahl commented Sep 27, 2016

@Acconut great read, thanks for passing that along. I was actually thinking the other day how that header could get a bit unwieldy. I am in support of only major version changes 👍 .

@Acconut
Copy link
Member Author

Acconut commented Sep 27, 2016

@bhstahl Thank you :)

@kvz
Copy link
Member

kvz commented Sep 30, 2016

He also mentions though that it's okay to

Make Minor and Patch Information Discoverable

I would vote to keep advertising the full SemVer (as that could also indicate to the client that it needs to watch out for bug X), but add to the protocol, that Servers and Clients MAY NOT reject service to major compatible versions (?)

This way we can rest assured that we can bump features and patches all we want, without removing the information completely, which could be useful to have in cases.

I think his main argument about only having major versions is when you put them in URLs, which means:

a whole new tree of resources

I can see why that is very bad, but that does not have to be the case with our protocol, if we are explicit about what to reject, and what not.

@ruv
Copy link

ruv commented Sep 30, 2016

I think his main argument about only having major versions is when you put them in URLs, which means:

a whole new tree of resources

Actually, Mark means not only URLs, a header can also by part of an identifier:

By “names,” I mean everything that’s used as an identifier, whether it’s a URL, a media type, a link relation name, HTTP header, whatever.

Regarding "watch out for bug X" — it is very partial solution since various bugs could be fixed not only in the protocol itself but mostly in the protocol implementations. And again, the corresponding citation from the Mark's post:

but that goes in the Server or User-Agent header respectively, and is completely separate from API versioning.

@kvz
Copy link
Member

kvz commented Oct 9, 2016

Actually, Mark means not only URLs, a header can also by part of an identifier

I stand corrected in what the author of this post meant 👍
That does make the post a bit less agreeable to me in that case. I think the issue is bigger with URLs as you get an immediate mismatch/404, just on the patch level already, if you're not working with some clever/ugly regex router. Headers are less of an issue in terms of deprecated whole resources, given the instructions around consumption are clear. A protocol is perfect for making such instructions binding.

Regarding "watch out for bug X" — it is very partial solution since various bugs could be fixed not only in the protocol itself but mostly in the protocol implementations. And again, the corresponding citation from the Mark's post

Sorry for not expressing myself more clearly, I didn't mean so much as an implementation bug, as a change in the protocol that is more backwards breaking in the real world than we had intended or foreseen given our limited ability to predict all edge cases. We'll try hard of course. But I think it's better to be on the safe side than to assume we'll have infallible judgement around this always.

If we publish a new minor release, which adds a new feature, we must make this new feature a new optional extension or else it would be a breaking change and this is not allowed in a minor release. Furthermore, versions can not be used as an indicator for which extensions are supported by the Server, as the are optional. For these situation, we introduced the Tus-Extension header which cares this exact information.

This leaves out the scenario where we might want to improve Core or existing Extensions in minor ways that do not break BC. Given the constraint that a MAJOR version bump risks deprecating the whole ecosystem of tus 1.x.x implementations, it's likely that we'll be inclined to slip in improvements as MINORs.

I agree that theoretically the Client and Server should not care about those as they are insignificant, so why disclose at all - but I'm worried in practice we might fail at this some day.

Perhaps there's something to better accommodate HTTP2, CORS, making Client retries mandatory, etc. I don't really care for these examples, the point here is more that there will likely be circumstances that warrant improvement to Core or an existing Extension, that we ship as MINOR as we agree it's not BC-breaking, until it turns out that it is. We'll fix it in a later release and mark v1.3.0 as one of our 💩 versions that should never be used. When the implementations disclose their full versions, we can at least make a 1.4.0 Server comes across a v.1.3.0 Client return a clear error message right off the bat, or choose to workaround whichever poor/contradicting behavior that protocol version encapsulated. It would also provide the analytics how many 1.3.0-speaking protocol clients are still around. In addition, we could deliver progressive enhancement, as a 1.8.0 Server could enable optional Core features introduced in >=1.5.0 Clients, while it does not attempt to do this for 1.4.x clients.

We'd throw all this away if we simplify the advertised version to just the MAJOR. Not looking at the last two digits is a lot less destructive then resetting them to 0, basically lying. There's no way to gather analytics or delicately handle oddities with old versions then.

If we'd change the proposal from not disclosing, to making it mandatory for Clients & Servers to just look at MAJOR and accept communication when it matches, I would easily +1.

@Acconut
Copy link
Member Author

Acconut commented Oct 6, 2019

This topic has sadly slipped off my radar for too long (mea culpa) but it is still as relevant.

@kvz: If we'd change the proposal from not disclosing, to making it mandatory for Clients & Servers to just look at MAJOR and accept communication when it matches, I would easily +1.

Just looking at the major version for determining compatibility is a great idea but would also be a breaking change IMO. Currently all server implementations require the Tus-Resumable header to be exactly 1.0.0. So if a tus 1.1 client tries to talk to a tus 1.0 server this leads to a conflict:

  • following the tus 1.0 specification, the 1.0-server will reject the 1.1-client because the client is not using a version supported by the server
  • following the tus 1.1 specification, the 1.1-client will advertise itself as tus-1.1-compatible since it expects the server to only look at the major version

In the end, the two components will not be able to communicate, so the tus 1.1 protocol would not be backwards compatible.

Another interesting aspect is that, unlike HTTP/1.0 and HTTP/1.1, there are no minor versions inside HTTP/2 (https://http2-explained.haxx.se/content/en/part5.html):

It was decided that clients and servers are either compatible with http2 or they are not. If a need arises to extend the protocol or modify things, then http3 will be born. There are no more minor versions in http2.

It doesn't entirely apply to tus, since we still want to evolve the protocol but I believe that introducing optional extensions still gives enough room for that.

All in all, I think that the versioning setup has been flawed from the beginning. There was little thought spent on how the client and server would negotiate versions and I am willing to take blame for that. But that's what we have to deal with right now, I guess. So my current opinion is to lock the Tus-Resumable header to 1.0.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants