Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[py] implement configurable configuration class for the http client #13286

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions py/selenium/webdriver/chrome/remote_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

from selenium.webdriver import DesiredCapabilities
from selenium.webdriver.chromium.remote_connection import ChromiumRemoteConnection
from selenium.webdriver.remote.client_config import ClientConfig


class ChromeRemoteConnection(ChromiumRemoteConnection):
Expand All @@ -28,11 +29,13 @@ def __init__(
remote_server_addr: str,
keep_alive: bool = True,
ignore_proxy: typing.Optional[bool] = False,
client_config: ClientConfig = None,
) -> None:
super().__init__(
remote_server_addr=remote_server_addr,
vendor_prefix="goog",
browser_name=ChromeRemoteConnection.browser_name,
keep_alive=keep_alive,
ignore_proxy=ignore_proxy,
client_config=client_config,
)
10 changes: 8 additions & 2 deletions py/selenium/webdriver/chromium/remote_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

from selenium.webdriver.remote.client_config import ClientConfig
from selenium.webdriver.remote.remote_connection import RemoteConnection


Expand All @@ -26,8 +26,14 @@ def __init__(
browser_name: str,
keep_alive: bool = True,
ignore_proxy: bool = False,
client_config: ClientConfig = None,
) -> None:
super().__init__(remote_server_addr, keep_alive, ignore_proxy)
super().__init__(
remote_server_addr=remote_server_addr,
keep_alive=keep_alive,
ignore_proxy=ignore_proxy,
client_config=client_config,
)
self.browser_name = browser_name
commands = self._remote_commands(vendor_prefix)
for key, value in commands.items():
Expand Down
10 changes: 10 additions & 0 deletions py/selenium/webdriver/common/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
# specific language governing permissions and limitations
# under the License.
import typing
import warnings
from abc import ABCMeta
from abc import abstractmethod
from enum import Enum
Expand Down Expand Up @@ -432,6 +433,15 @@ def add_argument(self, argument):
def ignore_local_proxy_environment_variables(self) -> None:
"""By calling this you will ignore HTTP_PROXY and HTTPS_PROXY from
being picked up and used."""
warnings.warn(
"using ignore_local_proxy_environment_variables in Options has been deprecated, "
"instead, create a Proxy instance with ProxyType.DIRECT to ignore proxy settings, "
"pass the proxy instance into a ClientConfig constructor, "
"pass the client config instance into the Webdriver constructor",
DeprecationWarning,
stacklevel=2,
)

self._ignore_local_proxy = True

def to_capabilities(self):
Expand Down
3 changes: 3 additions & 0 deletions py/selenium/webdriver/edge/remote_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

from selenium.webdriver import DesiredCapabilities
from selenium.webdriver.chromium.remote_connection import ChromiumRemoteConnection
from selenium.webdriver.remote.client_config import ClientConfig


class EdgeRemoteConnection(ChromiumRemoteConnection):
Expand All @@ -28,11 +29,13 @@ def __init__(
remote_server_addr: str,
keep_alive: bool = True,
ignore_proxy: typing.Optional[bool] = False,
client_config: ClientConfig = None,
) -> None:
super().__init__(
remote_server_addr=remote_server_addr,
vendor_prefix="goog",
browser_name=EdgeRemoteConnection.browser_name,
keep_alive=keep_alive,
ignore_proxy=ignore_proxy,
client_config=client_config,
)
17 changes: 15 additions & 2 deletions py/selenium/webdriver/firefox/remote_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,29 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
import typing

from selenium.webdriver.common.desired_capabilities import DesiredCapabilities
from selenium.webdriver.remote.client_config import ClientConfig
from selenium.webdriver.remote.remote_connection import RemoteConnection


class FirefoxRemoteConnection(RemoteConnection):
browser_name = DesiredCapabilities.FIREFOX["browserName"]

def __init__(self, remote_server_addr, keep_alive=True, ignore_proxy=False) -> None:
super().__init__(remote_server_addr, keep_alive, ignore_proxy)
def __init__(
self,
remote_server_addr: str,
keep_alive: bool = True,
ignore_proxy: typing.Optional[bool] = False,
client_config: ClientConfig = None,
) -> None:
super().__init__(
remote_server_addr=remote_server_addr,
keep_alive=keep_alive,
ignore_proxy=ignore_proxy,
client_config=client_config,
)

self._commands["GET_CONTEXT"] = ("GET", "/session/$sessionId/moz/context")
self._commands["SET_CONTEXT"] = ("POST", "/session/$sessionId/moz/context")
Expand Down
104 changes: 104 additions & 0 deletions py/selenium/webdriver/remote/client_config.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
# Licensed to the Software Freedom Conservancy (SFC) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The SFC licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
import os
from urllib import parse

from selenium.webdriver.common.proxy import Proxy
from selenium.webdriver.common.proxy import ProxyType


class ClientConfig:
def __init__(
self,
remote_server_addr: str,
keep_alive: bool = True,
proxy=None,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the argument type is missing. also the spacing does not look correct

) -> None:
self.remote_server_addr = remote_server_addr
self.keep_alive = keep_alive
self.proxy = proxy

@property
def remote_server_addr(self) -> str:
return self._remote_server_addr

@remote_server_addr.setter
def remote_server_addr(self, value: str):
self._remote_server_addr = value

@property
def keep_alive(self) -> bool:
""":Returns: The keep alive value."""
return self._keep_alive

@keep_alive.setter
def keep_alive(self, value: bool) -> None:
"""Toggles the keep alive value.

:Args:
- value: whether to keep the http connection alive
"""
self._keep_alive = value

@property
def proxy(self) -> Proxy:
""":Returns: The proxy used for communicating to the driver/server."""

self._proxy = self._proxy or Proxy(raw={"proxyType": ProxyType.SYSTEM})

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like an unexpected side effect to me. Would it make sense to set the default value in the constructor?

return self._proxy

@proxy.setter
def proxy(self, proxy: Proxy) -> None:
"""Provides the information for communicating with the driver or
server.

:Args:
- value: the proxy information to use to communicate with the driver or server
"""
self._proxy = proxy

def get_proxy_url(self):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we define the type of the returned value?

if self.proxy.proxy_type == ProxyType.DIRECT:
return None
elif self.proxy.proxy_type == ProxyType.SYSTEM:
_no_proxy = os.environ.get("no_proxy", os.environ.get("NO_PROXY"))
if _no_proxy:
for npu in _no_proxy.split(","):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we have better variable names? I have no idea what npu means :)

npu = npu.strip()
if npu == "*":
return None
n_url = parse.urlparse(npu)
remote_add = parse.urlparse(self.remote_server_addr)
if n_url.netloc:
if remote_add.netloc == n_url.netloc:
return None
else:
if n_url.path in remote_add.netloc:
return None
if self.remote_server_addr.startswith("https://"):
return os.environ.get("https_proxy", os.environ.get("HTTPS_PROXY"))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this type of call might be extracted to a separate method, so there is less unnecessary code duplication

if self.remote_server_addr.startswith("http://"):
return os.environ.get("http_proxy", os.environ.get("HTTP_PROXY"))
elif self.proxy.proxy_type == ProxyType.MANUAL:
if self.remote_server_addr.startswith("https://"):
return self.proxy.sslProxy
elif self.remote_server_addr.startswith("http://"):
return self.proxy.http_proxy
else:
return None
else:
return None
74 changes: 39 additions & 35 deletions py/selenium/webdriver/remote/remote_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import platform
import socket
import string
import warnings
from base64 import b64encode
from urllib import parse

Expand All @@ -29,6 +30,7 @@
from selenium import __version__

from . import utils
from .client_config import ClientConfig
from .command import Command
from .errorhandler import ErrorCode

Expand Down Expand Up @@ -211,12 +213,6 @@ def get_remote_connection_headers(cls, parsed_url, keep_alive=False):

return headers

def _get_proxy_url(self):
if self._url.startswith("https://"):
return os.environ.get("https_proxy", os.environ.get("HTTPS_PROXY"))
if self._url.startswith("http://"):
return os.environ.get("http_proxy", os.environ.get("HTTP_PROXY"))

def _identify_http_proxy_auth(self):
url = self._proxy_url
url = url[url.find(":") + 3 :]
Expand Down Expand Up @@ -248,31 +244,40 @@ def _get_connection_manager(self):

return urllib3.PoolManager(**pool_manager_init_args)

def __init__(self, remote_server_addr: str, keep_alive: bool = False, ignore_proxy: bool = False):
self.keep_alive = keep_alive
self._url = remote_server_addr

# Env var NO_PROXY will override this part of the code
_no_proxy = os.environ.get("no_proxy", os.environ.get("NO_PROXY"))
if _no_proxy:
for npu in _no_proxy.split(","):
npu = npu.strip()
if npu == "*":
ignore_proxy = True
break
n_url = parse.urlparse(npu)
remote_add = parse.urlparse(self._url)
if n_url.netloc:
if remote_add.netloc == n_url.netloc:
ignore_proxy = True
break
else:
if n_url.path in remote_add.netloc:
ignore_proxy = True
break

self._proxy_url = self._get_proxy_url() if not ignore_proxy else None
if keep_alive:
def __init__(
self,
remote_server_addr: str,
keep_alive: bool = True,
ignore_proxy: bool = False,
client_config: ClientConfig = None,
):
self._client_config = client_config or ClientConfig(remote_server_addr, keep_alive)

if remote_server_addr:
warnings.warn(
"setting keep_alive in RemoteConnection() is deprecated, " "set in ClientConfig instance insttead",
DeprecationWarning,
stacklevel=2,
)

if not keep_alive:
warnings.warn(
"setting keep_alive in RemoteConnection() is deprecated, " "set in ClientConfig instance insttead",
DeprecationWarning,
stacklevel=2,
)

if ignore_proxy:
warnings.warn(
"setting keep_alive in RemoteConnection() is deprecated, " "set in ClientConfig instance insttead",
DeprecationWarning,
stacklevel=2,
)
self._proxy_url = None
else:
self._proxy_url = self._client_config.get_proxy_url()

if self._client_config.keep_alive:
self._conn = self._get_connection_manager()
self._commands = remote_commands

Expand All @@ -296,7 +301,7 @@ def execute(self, command, params):
for word in substitute_params:
del params[word]
data = utils.dump_json(params)
url = f"{self._url}{path}"
url = f"{self._client_config.remote_server_addr}{path}"
return self._request(command_info[0], url, body=data)

def _request(self, method, url, body=None):
Expand All @@ -312,12 +317,11 @@ def _request(self, method, url, body=None):
"""
LOGGER.debug("%s %s %s", method, url, body)
parsed_url = parse.urlparse(url)
headers = self.get_remote_connection_headers(parsed_url, self.keep_alive)
response = None
headers = self.get_remote_connection_headers(parsed_url, self._client_config.keep_alive)
if body and method not in ("POST", "PUT"):
body = None

if self.keep_alive:
if self._client_config.keep_alive:
response = self._conn.request(method, url, body=body, headers=headers)
statuscode = response.status
else:
Expand Down