search

benoitc / hackney

simple HTTP client in Erlang
Other
1.34k stars 427 forks source link

Allow merging of SSL opts #655

Closed spydon closed 3 years ago

spydon commented 4 years ago

Related to #654 Since it isn't currently possible to merge your custom ssl opts with the default ones it could be a first step to expose the default ones so that the merging can be done client side.

benoitc commented 4 years ago

wouldn't exposing the partial_chain function just be enough?

spydon commented 4 years ago

It would work, but then the client still needs to duplicate the full list of default opts when setting custom ones.

spydon commented 4 years ago

What do you think @benoitc? And also, do you know why the checks are failing on some of the erlang versions?

benoitc commented 4 years ago

What do you think @benoitc? And also, do you know why the checks are failing on some of the erlang versions?

don't worry about the syntax issue. It's one of these usual bugs of Travis...

The issue with this change is that some people will still want maybe to use some other settings imo. Maybe instead of replacing ssl options we could merge them or provide a way to merge defaults and the one in options ? this could be done there: https://github.com/benoitc/hackney/blob/master/src/hackney_connection.erl#L125

Thoughts?

spydon commented 4 years ago

Yeah, that would be a better solution. But that would either be a breaking change, or to introduce a new method there? With the current change it will work the same as it does currently for those who want to use other settings, since the merging of the settings will need to be done client side and you would have to consciously call the new function to get the default opts.

spydon commented 4 years ago

@benoitc would something like this be better maybe?

spydon commented 4 years ago

Sorry to be a bother, have you had any time to look at this yet @benoitc? :)

benoitc commented 4 years ago

sorry for the late answer. That would probably work. I am doing a release today possibly including it :)

spydon commented 3 years ago

@benoitc is the plan to get this out on the 14th of November like it says on the milestone date? :)

spydon commented 3 years ago

@benoitc any plan for when the new release will be released?

benoitc commented 3 years ago

@spydon next release will happen next week on thursday. this feature should be part of it normally.

benoitc commented 3 years ago

@spydon next release will happen next week on thursday. this feature should be part of it normally.

that has been slightly delayed. It will happen this week, coming with a new documentation website.

spydon commented 3 years ago

Any updates on the release? :) We are seeing several teams having this problem now.

benoitc commented 3 years ago

Any updates on the release? :) We are seeing several teams having this problem now.

i will merge a minor release with it later today