SameerMesiah97 commented on code in PR #74921:
URL: https://github.com/apache/airflow/pull/74921#discussion_r4303081886
##########
providers/cncf/kubernetes/tests/unit/cncf/kubernetes/test_client.py:
##########
@@ -65,19 +74,86 @@ def test_load_config_ssl_ca_cert(self, conf, config):
@pytest.mark.platform("linux")
def test_enable_tcp_keepalive(self):
Review Comment:
Could you parametrize with default and non-default values for these:
TCP_KEEPIDLE
TCP_KEEPINTVL
TCP_KEEPCNT
##########
providers/cncf/kubernetes/tests/unit/cncf/kubernetes/test_client.py:
##########
@@ -65,19 +74,86 @@ def test_load_config_ssl_ca_cert(self, conf, config):
@pytest.mark.platform("linux")
def test_enable_tcp_keepalive(self):
- socket_options = [
+ configuration = Configuration()
+ configuration.socket_options = None
+
+ enable_tcp_keepalive(configuration)
+
+ assert configuration.socket_options == [
+ (socket.IPPROTO_TCP, socket.TCP_NODELAY, 1),
(socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1),
(socket.IPPROTO_TCP, socket.TCP_KEEPIDLE, 120),
(socket.IPPROTO_TCP, socket.TCP_KEEPINTVL, 30),
(socket.IPPROTO_TCP, socket.TCP_KEEPCNT, 6),
]
- expected_http_connection_options =
HTTPConnection.default_socket_options + socket_options
- expected_https_connection_options =
HTTPSConnection.default_socket_options + socket_options
- _enable_tcp_keepalive()
+ @pytest.mark.platform("linux")
+ def test_enable_tcp_keepalive_preserves_existing_options(self):
+ configuration = Configuration()
+ configuration.socket_options = [
+ (socket.SOL_SOCKET, socket.SO_RCVBUF, 1234),
+ (socket.SOL_SOCKET, socket.SO_KEEPALIVE, 0),
+ (socket.IPPROTO_TCP, socket.TCP_KEEPIDLE, 5),
+ ]
+
+ enable_tcp_keepalive(configuration)
+
+ assert configuration.socket_options == [
+ (socket.SOL_SOCKET, socket.SO_RCVBUF, 1234),
+ (socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1),
+ (socket.IPPROTO_TCP, socket.TCP_KEEPIDLE, 120),
+ (socket.IPPROTO_TCP, socket.TCP_KEEPINTVL, 30),
+ (socket.IPPROTO_TCP, socket.TCP_KEEPCNT, 6),
+ ]
+
+ @pytest.mark.platform("linux")
+ def test_enable_tcp_keepalive_is_idempotent(self):
+ configuration = Configuration()
+ configuration.socket_options = None
+
+ enable_tcp_keepalive(configuration)
+ first_call_options = list(configuration.socket_options)
+ enable_tcp_keepalive(configuration)
+
+ assert configuration.socket_options == first_call_options
+
+ @pytest.mark.platform("linux")
+ @pytest.mark.parametrize("enabled", [True, False])
+ @mock.patch("airflow.providers.cncf.kubernetes.kube_client.config",
autospec=True)
+ def test_get_kube_client_tcp_keepalive(self, mock_config, enabled):
+ with conf_vars({("kubernetes_executor", "enable_tcp_keepalive"):
str(enabled)}):
+ client = get_kube_client(in_cluster=False)
+
+ configuration_options = client.api_client.configuration.socket_options
+ pool_options =
client.api_client.rest_client.pool_manager.connection_pool_kw.get("socket_options")
+ keepalive = (socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1)
+ if enabled:
+ assert keepalive in configuration_options
+ assert keepalive in pool_options
+ else:
+ assert not configuration_options or keepalive not in
configuration_options
+ assert not pool_options or keepalive not in pool_options
+
+ @pytest.mark.platform("linux")
+ @pytest.mark.parametrize("enabled", [True, False])
+ def test_timeout_api_client_tcp_keepalive(self, enabled):
Review Comment:
Could you extend this with parametrization for disable_verify_ssl and
whether or not a configuration is supplied? Assert SSL verification and
keepalive behaviour, retaining the assertion that the global default
configuration is unchanged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
