Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions custom_model_runner/datarobot_drum/drum/entry_point.py
Original file line number Diff line number Diff line change
@@ -1,19 +1,17 @@
from datarobot_drum.drum.gunicorn.run_gunicorn import main_gunicorn
from datarobot_drum.drum.main import main
import sys
from datarobot_drum import RuntimeParameters
from datarobot_drum.drum.utils.setup import setup_options

from datarobot_drum.drum.enum import ArgumentsOptions

def run_drum_server():
cmd_args = sys.argv
is_server = False
if len(cmd_args) > 1 and cmd_args[1] == "server":
is_server = True

def run_drum_server():
options = setup_options()
if (
RuntimeParameters.has("DRUM_SERVER_TYPE")
options.subparser_name == ArgumentsOptions.SERVER
and RuntimeParameters.has("DRUM_SERVER_TYPE")
and str(RuntimeParameters.get("DRUM_SERVER_TYPE")).lower() == "gunicorn"
and is_server
):
main_gunicorn()
else:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
if temp_loglevel in {"debug", "info", "warning", "error", "critical"}:
loglevel = temp_loglevel

bind = os.environ.get("ADDRESS", "0.0.0.0:8080")
bind = os.environ["ADDRESS"]
# loglevel = "info"
accesslog = "-"
access_log_format = '%(h)s %(l)s %(u)s %(t)s "%(r)s" %(s)s %(b)s "%(f)s" "%(a)s"'
Expand Down
7 changes: 6 additions & 1 deletion custom_model_runner/datarobot_drum/drum/utils/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
Released under the terms of DataRobot Tool and Utility Agreement.
"""

import os
import sys

from datarobot_drum import RuntimeParameters
Expand All @@ -31,7 +32,6 @@ def setup_options(args=None):
Parsed command line options as an argparse.Namespace object.
"""
arg_parser = CMRunnerArgsRegistry.get_arg_parser()

try:
import argcomplete
except ImportError:
Expand All @@ -48,6 +48,11 @@ def setup_options(args=None):

options = arg_parser.parse_args(args)

# Override env var from CLI parameter.
# Now only ADDRESS is supported.
if getattr(options, "address", None):
os.environ["ADDRESS"] = options.address
Copy link

Choose a reason for hiding this comment

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

Bug: Empty address string causes KeyError in gunicorn config

The condition if getattr(options, "address", None) uses truthiness checking, which evaluates to False for empty strings. If a user provides an empty address like --address "", the ADDRESS environment variable won't be set. Combined with the removal of the default fallback in gunicorn.conf.py (changing from os.environ.get("ADDRESS", "0.0.0.0:8080") to os.environ["ADDRESS"]), this causes a KeyError when gunicorn starts. Consider using an explicit is not None check instead of truthiness.

Additional Locations (1)

Fix in Cursor Fix in Web

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are different sub parsers for drum:
drum score
drum server
address is not defined for all of them, so in this case I keep the current check.


"""Set max workers from runtime parameters if available."""
if RuntimeParameters.has("CUSTOM_MODEL_WORKERS"):
options.max_workers = RuntimeParameters.get("CUSTOM_MODEL_WORKERS")
Expand Down
20 changes: 10 additions & 10 deletions tests/functional/test_runtime_parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,8 @@ def _test_custom_model_with_runtime_params(
if docker:
cmd += " --docker {} --verbose ".format(docker)

_, stdout, _ = _exec_shell_cmd(cmd, err_msg=None, assert_if_fail=False, env=env)
return stdout
_, stdout, stderr = _exec_shell_cmd(cmd, err_msg=None, assert_if_fail=False, env=env)
return stdout, stderr

@classmethod
def _setup_runtime_parameters(
Expand Down Expand Up @@ -251,20 +251,20 @@ def _setup_runtime_parameters(
def test_runtime_parameters_invalid_yaml(
self, resources, tmp_path, runtime_param_values_stream, use_runtime_params_env_var
):
stdout = self._test_custom_model_with_runtime_params(
stdout, stderr = self._test_custom_model_with_runtime_params(
resources,
tmp_path,
runtime_param_values_stream,
is_invalid_yaml=True,
use_runtime_params_env_var=use_runtime_params_env_var,
)
assert "Invalid runtime parameter values YAML content!" in stdout
assert "Invalid runtime parameter values YAML content!" in stdout + stderr

@pytest.mark.parametrize("use_runtime_params_env_var", [True, False])
def test_runtime_parameters_missing_attr(
self, resources, tmp_path, runtime_param_values_stream, use_runtime_params_env_var
):
stdout = self._test_custom_model_with_runtime_params(
stdout, stderr = self._test_custom_model_with_runtime_params(
resources,
tmp_path,
runtime_param_values_stream,
Expand All @@ -273,27 +273,27 @@ def test_runtime_parameters_missing_attr(
)
assert re.search(
r".*Failed to load runtime parameter.*{\\'credentialType\\': DataError\(\\'is required\\'\)}.*",
stdout,
stdout + stderr,
)

def test_runtime_parameters_boolean_invalid(
self, resources, tmp_path, runtime_param_values_stream
):
stderr = self._test_custom_model_with_runtime_params(
stdout, stderr = self._test_custom_model_with_runtime_params(
resources, tmp_path, runtime_param_values_stream, bool_var_value="text"
)
assert re.search(
r".*Failed to load runtime parameter.*value should be True or False.*",
stderr,
stdout + stderr,
)

def test_runtime_parameters_numeric_invalid(
self, resources, tmp_path, runtime_param_values_stream
):
stderr = self._test_custom_model_with_runtime_params(
stdout, stderr = self._test_custom_model_with_runtime_params(
resources, tmp_path, runtime_param_values_stream, numeric_var_value="text"
)
assert re.search(
r".*Failed to load runtime parameter.*value can.*t be converted to float.*",
stderr,
stdout + stderr,
)
Loading