Correct Net::HTTP::Persistent bug when using newer versions #8387
Conversation
This change moves the SSL option configuration out of `http` and into `ssl_options`. Doing so allows subclasses to create their own SSL config, without having to clone `http` (and thus leading to less ongoing maintenance). Primarily, this change allows for the `Net::HTTP::Persistent` shim to only set `use_ssl` to true where needed; Newer releases of the library manage their own SSL config.
|
@twalpole / @p0deje weren't we considering deprecating/removing this class since we updated the default client to use keep-alive, which is the functionally equivalent way to do what Persistent does for HTTP v2? I think this class is only needed if HTTP2 isn't supported, which should be nothing at this point? Let me know if I'm off, I can't really remember our last conversation on this. If we aren't removing the class, this code looks good to me. :) |
|
@DylanLacey Persistent connections should work in the default client starting from 7c9ca8e. Do you not have it working and you need to use net-http-persistent? |
|
Hah, just noticed that we didn't have a conversation about removing it, I just suggested it and didn't follow up: #7065 (comment) |
|
Either of these are fine by me! I didn't note the change to the standard HTTP client and since the Persistent shim was still present but not working... Shall I leave this open for a week or so in case anyone else is watching the issue tracker with rabid interest for Persistent improvements and has an argument for keeping it? |
|
I believe we should proceed with deprecating this class in favor of default HTTP client for 4.0. |
|
And deprecated: f6d831b |
Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.
Description
Fix the integration with the Net HTTP Persistent Gem.
Motivation and Context
The default Net::HTTP Library allows the end user to force SSL using the
use_ssl=method. Net::HTTP::Persistent doesn't in newer releases. This causes errors to be thrown when using the Net::HTTP::Persistent shim.This change moves SSL configuration into its own method, allowing the shim to handle this for different versions while keeping the default HTTP code clean.
Types of changes
Checklist