Versioned API for Conduit_async_ssl

[rgrinberg]

The recent improvements by @vbmithr made me a bit worried about all the downstream work we'd be creating ourselves and our users by just changing the API so frequently. To try and address this, I propose that we version Conduit_async's API like I've done this in PR. Unfortunately, even versioning the API will require a bit of breakage as we must switch module aliases for proper versioning. Here's the versioning scheme in a nutshell:

• The interfaces for each version are defined in Conduit_async.S. Those need to exist here because the real and fake implementations don't share the same types.

• All SSL conditional code will live in private_ssl_{real,fake}.ml. This is like a staging ground for the building blocks used for the actual api.

• The actual API will be implemented in v1.ml, v2.ml etc.

The current situation with v1 and v2:

V1 superficially resembles the current API. So users that don't want to migrate their code will need to do open Conduit_async_ssl.V1 and tweak some module names.

V2 includes the the yet to be released API with new features introduces by @vbmithr . I suggest that he works on this version further until he's satisfied.

If this approach proves to be a success, I suggest that adopt it elsewhere in conduit.

[rgrinberg]

The last commit should give full compat for v1 at the expense of gross nesting. To use v1 without changing any of your existing code, just do open Conduit_async.V1.

To use V2, the recommended approach is to do module Conduit = Conduit_async.V2

[vbmithr]

Ok with the general approach. I suggest V2 to be a cutting edge API that can break. So whenever there are changes to be introduced, I can do it freely. And V1 would stay as conservative as possible, that kind of thing.

[samoht]

I am a bit worried of doing more than the minimal amount of maintenance work on conduit nowadays.

I think that this library needs a complete rewrite, with a better architecture which will avoid all the cppo steps. See mirage/mirage#882 for more context. So I am reluctant to make any big API change until the new architecture is settled.

[vbmithr]

Agree that conduit and cohttp need to be rewritten 🍡

[rgrinberg]

Well I think that any new architecture should consider versioning very careful. Otherwise we'll be stuck in the same situation as we are now. Unable to evolve the API without massive pain. Note that I'm not making any drastic changes to the API, I'm only shrink wrapping to guarantee compatibility. As witnessed by @vbmithr's latest changes, we do need to evolve the API anyhow. So let's figure out a way to do it while minimizing downstream pain.

@vbmithr I think it's best to keep all the interfaces forward compatible. If you'd like to make a breaking change, just plan it properly and introduce V3. Adding proper type equalities between the interfaces, and properly sharing implementation via private modules will guarantee that this will be painless.

[samoht]

I don't understand why we can't evolve the API? What is the difference between conduit and other libraires? Why do we need to encode versions in the library name instead of using opam constraints?

[samoht]

e.g. feel free to break/fix the API and bump the version numbers, and fix the revdeps. That's what we do in other libraires :-)

[rgrinberg]

Well this approach is far easier in my opinion. We don't need to invite users to yet another round of churn every time we want to tweak the API's. Nobody has time for this and there are also very little benefits. Especially that another round of breaking changes is around the corner.

Quite often, there's really nothing wrong with the old API's that we have (like this in case). So I just don't see why we should ask everyone to update their code, when it's really quite painless for us to support the old API.

[rgrinberg]

@vbmithr if you really want to live on the bleeding edge we could always add module V_latest = Vn and keep that updated.

[rgrinberg]

I don't understand why we can't evolve the API? What is the difference between conduit and other libraires? Why do we need to encode versions in the library name instead of using opam constraints?

Conduit is in a bit of a unique position. It tracks some very important upstream dependencies, has many downstream users, and itself is a very small API. This puts its users in many crappy situations like "I can't update async because that will break conduit, but I need the latest conduit for the latest version of cohttp that includes my bug fix". I think that these small iterations to the conduit API don't warrant the pain they inflict on the user.

[vbmithr]

The problem with the current API is that it's limiting.
SSL support with no authentication and no client certificate support is toy-grade.
I don't really care of APIs personally, I just want to be able to do stuff I need with it.
If this library is a massive hub and has lots of dependencies… why not decide on a set of features and implement 100%, and then don't touch in a long time…?

[rgrinberg]

I think that's what we intend to do but invariably the following happens: some parts of the implementation remain incomplete and new feature requests pop up. That's ok, and i don't think anyone is at fault here. I think versioning will be quite useful for your goal here Vincent. Just implement whatever ssl api you think is best and don't worry about backwards compat. Just don't break the old code unless you have a really really good reason. Even if today's API is toy grade, I can tell you that it runs in production in quite a few places. A versioned API sounds like having your cake and eating it too, but I want to emphasize that it only works for very interfaces like conduit's. I don't think this approach scales everywhere.

…

On Fri, Jun 8, 2018, 4:54 PM Vincent Bernardoff ***@***.***> wrote: The problem with the current API is that it's limiting. SSL support with no authentication and no client certificate support is toy-grade. I don't really care of APIs personally, I just want to be able to do stuff I need with it. If this library is a massive hub and has lots of dependencies… why not decide on a set of features and implement 100%, and then don't touch in a long time…? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#265 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIe-wZDUHWIkEG3RNDEfh9uWVXlXjZuks5t6kndgaJpZM4UfvC-> .

[samoht]

Something that I don't get is why do we need to version/break every users. Can't we just version the Async/SSL API if you think this one still needs to evolve a bit? Why does it even have to impact user of vchan/lwt for instance?

[samoht]

Hum I think I need more coffee, you actually are only version the async/ssl API. As I am not a user/developer/maintainer of this part, I don't have a strong opinion on this part. But as you will be asking for downstream users to update anyway to add V1 to their code, as I don't see the issue as asking them to use the new improved API instead.

[rgrinberg]

I don't know about the vchan API but I would certainly like the lwt API to evolve as well. If you don't think versioning the lwt API warranted, I certainly won't press for it as I'm nowhere near the only maintainer of it.

…

On Fri, Jun 8, 2018, 5:08 PM Thomas Gazagnaire ***@***.***> wrote: Something that I don't get is why do we need to version/break every user. Can't we just version the Async/SSL API if you think this one still needs to evolve a bit? Why does it even have to impact user of vchan/lwt for instance? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#265 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIe-3092mazWYZZrXAdBxlcXEYbX_WBks5t6k0cgaJpZM4UfvC-> .

[rgrinberg]

But as you will be asking for downstream users to update anyway to add V1 to their code, as I don't see the issue as asking them to use the new improved API instead.

Yes, but that's unfortunately unavoidable because there's sometimes no way to go from (wrapped false) to (wrapped true) without some breakage. Btw, I could also just include V1 in Conduit_async and that will make things even more compatible. I'm a bit on the fence here, but I will probably do it.

I could ask them to use the new API, but I would rather leave the decision to them. Especially that the new API has no benefits to them if they aren't using any of the new config options. Also, I strongly suspect that there will be more improvements down the line, updates to keep up with async, async_ssl, core, ppx, conduit itself. Creating this versioning scheme took me a few hours, and I suspect it will save me more time than that down the line. And that's completely ignoring the time it will save for users.

[vbmithr]

On 08/06/2018 12:05, Rudi Grinberg wrote:

I think that's what we intend to do but invariably the following happens:
some parts of the implementation remain incomplete and new feature requests
pop up. That's ok, and i don't think anyone is at fault here.

I think versioning will be quite useful for your goal here Vincent. Just
implement whatever ssl api you think is best and don't worry about
backwards compat. Just don't break the old code unless you have a really
really good reason. Even if today's API is toy grade, I can tell you that
it runs in production in quite a few places.

Like in Tezos for example. That's why I was complaining of the lack of client
certificate support and authentication support. Especially since tls supports
those.

Personally I don't care about API breakage. I favor features and
quality/simplicity of the API over breakage.

Yeah I think the Lwt version is used a lot more than then Async version. There
might be some Async users. I was fine with your proposal anyway from the
beginning. Because as long as the latest version of Conduit/Async does the job
for me I'm happy.

A versioned API sounds like having your cake and eating it too, but I want
to emphasize that it only works for very interfaces like conduit's. I don't
think this approach scales everywhere.

Yeah, I personally think that one day we should start a project to rewrite
Cohttp from scratch.

Anyway, that's why I am fine with your proposal. Thanks for taking the time to
think about it and to propose a solution.

[rgrinberg]

Like in Tezos for example. That's why I was complaining of the lack of client
certificate support and authentication support. Especially since tls supports
those.

Doesn't tezos use Lwt?

Yeah, I personally think that one day we should start a project to rewrite
Cohttp from scratch.

I disagree with this because I already see httpaf accomplishing this goal. If we break cohttp's api, then it really has no reason to exist. Users who care about performance should just use httpaf.

[rgrinberg]

@vbmithr the latest commit fully restores compat for existing users with a deprecation notice. Feel free to make further improvements to V2.

[hannesm]

FWIW I agree with @samoht and am afraid of maintaining "Versioned API" (as we discussed extensively when releasing MirageOS3 and dropping/revising the old modules, even the V1/V1_LWT names), esp. since we already have versions on the opam package level. When revising APIs, my intention is to make them clearer -- and get rid of old code, which is in stark contrast to keep backwards compatibility.

According to http://opam.ocaml.org/packages/conduit-async/conduit-async.1.1.0/ conduit-async has a single user (while conduit has 17), I really don't see the point of introducing "Versioned API" for this.