[py] Use Config class for http client settings#12033
[py] Use Config class for http client settings#12033titusfortner wants to merge 3 commits intotrunkfrom
Conversation
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## trunk #12033 +/- ##
==========================================
+ Coverage 54.85% 54.91% +0.06%
==========================================
Files 86 87 +1
Lines 5728 5789 +61
Branches 233 232 -1
==========================================
+ Hits 3142 3179 +37
- Misses 2353 2378 +25
+ Partials 233 232 -1
☔ View full report in Codecov by Sentry. |
c0903b6 to
ffb4a84
Compare
…nnection does not have a browser name
15564a9 to
b4fb1e1
Compare
b4fb1e1 to
8602eaa
Compare
|
Initial pass through and it looks ok |
|
This needs rebasing after other PR merged |
|
|
||
| def __init__(self, remote_server_addr: str, keep_alive: bool = False, ignore_proxy: bool = False): | ||
| self.keep_alive = keep_alive | ||
| def __init__(self, remote_server_addr: str, client_config: ClientConfig = ClientConfig()): |
There was a problem hiding this comment.
So in Python, any object that is a function default is "cached", unlike Ruby, where client_config will always get a new instance of ClientConfig.
This may affect scripts that run multiple browsers in one session, if they don't set a different default client config.
There was a problem hiding this comment.
So the better pattern is to use None as default and then set it in the constructor body?
|
I like this idea of a config class. I would like an easier interface to swap out |
|
Well, I'm still not sure the right way to get browser specific features in a remote session. Someone did some magic at one point, but then I think there was an issue. I never followed up. |
|
I'm breaking this up into pieces for my sanity and to make sure I'm doing this properly. |
Description
ClientConfigclass with:keep_alivedeprecation changed (it was never supported in Service class)ignore_proxydeprecated in Options (not an option)Motivation and Context
Python implementation of
command_executor/RemoteConnectionis similar to Java. A user can create the class themselves, but can only use it as a parameter forRemoteWebDriver. Local drivers are subclassed and have theirRemoteConnectionclasses created for them.Java addressed this with the
ClientConfigclass. (Ruby has always had a separate http_client, and I haven't looked at what JS or .NET do).The only way to use a proxy right now is with environment variables. If a user wants to ignore the environment variables they use
ignore_local_proxy()onOptionsclass.ClientConfigtakes aProxyinstance so the user does not need to set environment variables. Instead of having the user passignore_proxy: true, the user would passproxy: Proxy({"ProxyType": ProxyType.DIRECT}). If that seems too odd, I can add back the ignore_proxy parameter, but only having the "right" proxy object seems more "correct."Example usage: