Skip to content

Commit

Permalink
Backport PR #910 on branch 1.x (Add back support for kernel launch ti…
Browse files Browse the repository at this point in the history
…meout pad) (#911)

Co-authored-by: Ciprian Anton <ciprian.anton@ni.com>
  • Loading branch information
meeseeksmachine and CiprianAnton authored Jul 7, 2022
1 parent add31bb commit 04b197d
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 12 deletions.
39 changes: 32 additions & 7 deletions jupyter_server/gateway/gateway_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ def connect_timeout_default(self):
os.environ.get("JUPYTER_GATEWAY_CONNECT_TIMEOUT", self.connect_timeout_default_value)
)

request_timeout_default_value = 40.0
request_timeout_default_value = 42.0
request_timeout_env = "JUPYTER_GATEWAY_REQUEST_TIMEOUT"
request_timeout = Float(
default_value=request_timeout_default_value,
Expand Down Expand Up @@ -341,6 +341,25 @@ def gateway_retry_max_default(self):
os.environ.get("JUPYTER_GATEWAY_RETRY_MAX", self.gateway_retry_max_default_value)
)

launch_timeout_pad_default_value = 2.0
launch_timeout_pad_env = "JUPYTER_GATEWAY_LAUNCH_TIMEOUT_PAD"
launch_timeout_pad = Float(
default_value=launch_timeout_pad_default_value,
config=True,
help="""Timeout pad to be ensured between KERNEL_LAUNCH_TIMEOUT and request_timeout
such that request_timeout >= KERNEL_LAUNCH_TIMEOUT + launch_timeout_pad.
(JUPYTER_GATEWAY_LAUNCH_TIMEOUT_PAD env var)""",
)

@default("launch_timeout_pad")
def launch_timeout_pad_default(self):
return float(
os.environ.get(
"JUPYTER_GATEWAY_LAUNCH_TIMEOUT_PAD",
self.launch_timeout_pad_default_value,
)
)

@property
def gateway_enabled(self):
return bool(self.url is not None and len(self.url) > 0)
Expand All @@ -353,12 +372,18 @@ def init_static_args(self):
perform this operation once.
"""
# 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 that request timeout and KERNEL_LAUNCH_TIMEOUT are in sync, taking the
# greater value of the two and taking into account the following relation:
# request_timeout = KERNEL_LAUNCH_TIME + padding
minimum_request_timeout = (
float(GatewayClient.KERNEL_LAUNCH_TIMEOUT) + self.launch_timeout_pad
)
if self.request_timeout < minimum_request_timeout:
self.request_timeout = minimum_request_timeout
elif self.request_timeout > minimum_request_timeout:
GatewayClient.KERNEL_LAUNCH_TIMEOUT = int(
self.request_timeout - self.launch_timeout_pad
)
# Ensure any adjustments are reflected in env.
os.environ["KERNEL_LAUNCH_TIMEOUT"] = str(GatewayClient.KERNEL_LAUNCH_TIMEOUT)

Expand Down
41 changes: 36 additions & 5 deletions tests/test_gateway.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ def init_gateway(monkeypatch):
monkeypatch.setenv("JUPYTER_GATEWAY_HTTP_USER", mock_http_user)
monkeypatch.setenv("JUPYTER_GATEWAY_REQUEST_TIMEOUT", "44.4")
monkeypatch.setenv("JUPYTER_GATEWAY_CONNECT_TIMEOUT", "44.4")
monkeypatch.setenv("JUPYTER_GATEWAY_LAUNCH_TIMEOUT_PAD", "1.1")
yield
GatewayClient.clear_instance()

Expand All @@ -198,11 +199,10 @@ async def test_gateway_env_options(init_gateway, jp_serverapp):
jp_serverapp.gateway_config.connect_timeout == jp_serverapp.gateway_config.request_timeout
)
assert jp_serverapp.gateway_config.connect_timeout == 44.4
assert jp_serverapp.gateway_config.launch_timeout_pad == 1.1

GatewayClient.instance().init_static_args()
assert GatewayClient.instance().KERNEL_LAUNCH_TIMEOUT == int(
jp_serverapp.gateway_config.request_timeout
)
assert GatewayClient.instance().KERNEL_LAUNCH_TIMEOUT == 43


async def test_gateway_cli_options(jp_configurable_serverapp):
Expand All @@ -211,6 +211,7 @@ async def test_gateway_cli_options(jp_configurable_serverapp):
"--GatewayClient.http_user=" + mock_http_user,
"--GatewayClient.connect_timeout=44.4",
"--GatewayClient.request_timeout=96.0",
"--GatewayClient.launch_timeout_pad=5.1",
]

GatewayClient.clear_instance()
Expand All @@ -221,10 +222,40 @@ async def test_gateway_cli_options(jp_configurable_serverapp):
assert app.gateway_config.http_user == mock_http_user
assert app.gateway_config.connect_timeout == 44.4
assert app.gateway_config.request_timeout == 96.0
assert app.gateway_config.launch_timeout_pad == 5.1
GatewayClient.instance().init_static_args()
assert (
GatewayClient.instance().KERNEL_LAUNCH_TIMEOUT == 96
) # Ensure KLT gets set from request-timeout
GatewayClient.instance().KERNEL_LAUNCH_TIMEOUT == 90
) # Ensure KLT gets set from request-timeout - launch_timeout_pad
GatewayClient.clear_instance()


@pytest.mark.parametrize(
"request_timeout,kernel_launch_timeout,expected_request_timeout,expected_kernel_launch_timeout",
[(50, 10, 50, 45), (10, 50, 55, 50)],
)
async def test_gateway_request_timeout_pad_option(
jp_configurable_serverapp,
monkeypatch,
request_timeout,
kernel_launch_timeout,
expected_request_timeout,
expected_kernel_launch_timeout,
):
argv = [
f"--GatewayClient.request_timeout={request_timeout}",
"--GatewayClient.launch_timeout_pad=5",
]

GatewayClient.clear_instance()
app = jp_configurable_serverapp(argv=argv)

monkeypatch.setattr(GatewayClient, "KERNEL_LAUNCH_TIMEOUT", kernel_launch_timeout)
GatewayClient.instance().init_static_args()

assert app.gateway_config.request_timeout == expected_request_timeout
assert GatewayClient.instance().KERNEL_LAUNCH_TIMEOUT == expected_kernel_launch_timeout

GatewayClient.clear_instance()


Expand Down

0 comments on commit 04b197d

Please sign in to comment.