CP-36097 ocaml-conduit patch#560
Conversation
|
|
||
| +module Overrides : sig | ||
| + module Client : sig | ||
| + type t = { ctx : [ `Ssl_not_available ] option ; hostname : string option } |
There was a problem hiding this comment.
This is using polymorphic variants. Is it not possible regular variants which create better error messages?
There was a problem hiding this comment.
I believe it needs to be this otherwise it won't compile (the library uses polymorphic variants everywhere)
psafont
left a comment
psafont
left a comment
There was a problem hiding this comment.
The version updates look good to me. The patches are applied to all the packages related to the repository.
I'm not an expert in conduit, the changes in there looked sensible to me.
I wanted to note that the patch doesn't seem to block us from using conduit 4.0
I think it might be worth pinging the conduit maintainers to see their thoughts on the approach.
Signed-off-by: Ben Anson <ben.anson@citrix.com>
Signed-off-by: Ben Anson <ben.anson@citrix.com>
|
Can we merge this? |
|
Just a curiosity, have you tried to see if there is a chance to get this feature upstreamed in coduit/cohttp for the 5+ releases? |
Hi there Marcello - I opened a PR upstream a while ago mirage/ocaml-conduit#390. Would it be sufficient to get it into master, or would I have to do each of the backports myself? |
|
That's good, at least to try and start some discussions. Next time we have the cohttp meeting I will add it to the list of things to discuss |
|
Thanks! I'll tidy up the upstream PR a bit to make sure it's in a fit state |
4 participants
Motivation for the patch is so that we can connect to a particular hostname/IP, but specify something else to verify against (SNI)