From 7de8df4dd80f4da03066ddb498d1541182ad19a8 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Mon, 6 Apr 2020 16:40:56 -0700 Subject: [PATCH] Gateway only: Ensure launch and request timeouts are in sync (#5317) Prior to this change, the request timeout for a Gateway request was synchronized with KERNEL_LAUNCH_TIMEOUT only if KLT was greater. However, the two are closely associated and KLT should be adjusted if the configurable request_timeout is greater. This change ensures that the two values are synchronized to the greater value. It changes the two configurable timeouts to default to 40 (to match that of KLT) and removes the 2-second pad, since that wasn't helpful and only confused the situation. These changes were prompted by this issue: jupyter/enterprise_gateway#792 --- notebook/gateway/managers.py | 18 ++++++++++-------- notebook/tests/test_gateway.py | 15 +++++++++++---- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/notebook/gateway/managers.py b/notebook/gateway/managers.py index d49960753..fb53f044a 100644 --- a/notebook/gateway/managers.py +++ b/notebook/gateway/managers.py @@ -102,7 +102,7 @@ class GatewayClient(SingletonConfigurable): def _kernelspecs_resource_endpoint_default(self): return os.environ.get(self.kernelspecs_resource_endpoint_env, self.kernelspecs_resource_endpoint_default_value) - connect_timeout_default_value = 60.0 + connect_timeout_default_value = 40.0 connect_timeout_env = 'JUPYTER_GATEWAY_CONNECT_TIMEOUT' connect_timeout = Float(default_value=connect_timeout_default_value, config=True, help="""The time allowed for HTTP connection establishment with the Gateway server. @@ -112,7 +112,7 @@ class GatewayClient(SingletonConfigurable): def connect_timeout_default(self): return float(os.environ.get('JUPYTER_GATEWAY_CONNECT_TIMEOUT', self.connect_timeout_default_value)) - request_timeout_default_value = 60.0 + request_timeout_default_value = 40.0 request_timeout_env = 'JUPYTER_GATEWAY_REQUEST_TIMEOUT' request_timeout = Float(default_value=request_timeout_default_value, config=True, help="""The time allowed for HTTP request completion. (JUPYTER_GATEWAY_REQUEST_TIMEOUT env var)""") @@ -226,18 +226,20 @@ class GatewayClient(SingletonConfigurable): # Ensure KERNEL_LAUNCH_TIMEOUT has a default value. KERNEL_LAUNCH_TIMEOUT = int(os.environ.get('KERNEL_LAUNCH_TIMEOUT', 40)) - os.environ['KERNEL_LAUNCH_TIMEOUT'] = str(KERNEL_LAUNCH_TIMEOUT) - - LAUNCH_TIMEOUT_PAD = int(os.environ.get('LAUNCH_TIMEOUT_PAD', 2)) def init_static_args(self): """Initialize arguments used on every request. Since these are static values, we'll perform this operation once. """ - # Ensure that request timeout is at least "pad" greater than launch timeout. - if self.request_timeout < float(GatewayClient.KERNEL_LAUNCH_TIMEOUT + GatewayClient.LAUNCH_TIMEOUT_PAD): - self.request_timeout = float(GatewayClient.KERNEL_LAUNCH_TIMEOUT + GatewayClient.LAUNCH_TIMEOUT_PAD) + # Ensure that request timeout and KERNEL_LAUNCH_TIMEOUT are the same, taking the + # greater value of the two. + if self.request_timeout < float(GatewayClient.KERNEL_LAUNCH_TIMEOUT): + self.request_timeout = float(GatewayClient.KERNEL_LAUNCH_TIMEOUT) + elif self.request_timeout > float(GatewayClient.KERNEL_LAUNCH_TIMEOUT): + GatewayClient.KERNEL_LAUNCH_TIMEOUT = int(self.request_timeout) + # Ensure any adjustments are reflected in env. + os.environ['KERNEL_LAUNCH_TIMEOUT'] = str(GatewayClient.KERNEL_LAUNCH_TIMEOUT) self._static_args['headers'] = json.loads(self.headers) if 'Authorization' not in self._static_args['headers'].keys(): diff --git a/notebook/tests/test_gateway.py b/notebook/tests/test_gateway.py index 116b3db67..bc42014e4 100644 --- a/notebook/tests/test_gateway.py +++ b/notebook/tests/test_gateway.py @@ -117,8 +117,8 @@ def mock_gateway_request(url, **kwargs): raise gen.Return(response) # Fetch existing kernel - if endpoint.rfind('/api/kernels/') >= 0 and method == 'GET': - requested_kernel_id = endpoint.rpartition('/')[2] + if endpoint.rfind('/api/kernels/') >= 0 and method == 'GET': + requested_kernel_id = endpoint.rpartition('/')[2] if requested_kernel_id in running_kernels: response_buf = StringIO(json.dumps(running_kernels.get(requested_kernel_id))) response = yield maybe_future(HTTPResponse(request, 200, buffer=response_buf)) @@ -149,21 +149,28 @@ class TestGateway(NotebookTestBase): def get_patch_env(cls): test_env = super(TestGateway, cls).get_patch_env() test_env.update({'JUPYTER_GATEWAY_URL': TestGateway.mock_gateway_url, - 'JUPYTER_GATEWAY_REQUEST_TIMEOUT': '44.4'}) + 'JUPYTER_GATEWAY_CONNECT_TIMEOUT': '44.4'}) return test_env @classmethod def get_argv(cls): argv = super(TestGateway, cls).get_argv() - argv.extend(['--GatewayClient.connect_timeout=44.4', '--GatewayClient.http_user=' + TestGateway.mock_http_user]) + argv.extend(['--GatewayClient.request_timeout=96.0', '--GatewayClient.http_user=' + TestGateway.mock_http_user]) return argv + def setUp(self): + kwargs = dict() + GatewayClient.instance().load_connection_args(**kwargs) + super(TestGateway, self).setUp() + def test_gateway_options(self): nt.assert_equal(self.notebook.gateway_config.gateway_enabled, True) nt.assert_equal(self.notebook.gateway_config.url, TestGateway.mock_gateway_url) nt.assert_equal(self.notebook.gateway_config.http_user, TestGateway.mock_http_user) nt.assert_equal(self.notebook.gateway_config.connect_timeout, self.notebook.gateway_config.connect_timeout) nt.assert_equal(self.notebook.gateway_config.connect_timeout, 44.4) + nt.assert_equal(self.notebook.gateway_config.request_timeout, 96.0) + nt.assert_equal(os.environ['KERNEL_LAUNCH_TIMEOUT'], str(96)) # Ensure KLT gets set from request-timeout def test_gateway_class_mappings(self): # Ensure appropriate class mappings are in place.