removes docker container timeout

better handled by having exceptions properly handled now

Author: tomjemmett

src/nhp/docker/__main__.py

@@ -3,18 +3,12 @@
 import argparse
 import logging
 import os
-import threading
 
 from nhp.docker.config import Config
 from nhp.docker.run import RunWithAzureStorage, RunWithLocalStorage
 from nhp.model.run import run_all
 
 
-def _exit_container():
-    logging.error("\nTimed out, killing container")
-    os._exit(2)
-
-
 def parse_args():
     """Parse command line arguments."""
     parser = argparse.ArgumentParser()
@@ -53,7 +47,6 @@ def main(config: Config = Config()):
         runner = RunWithAzureStorage(args.params_file, config)
 
     logging.info("running model for: %s", args.params_file)
-    logging.info("container timeout: %ds", config.CONTAINER_TIMEOUT_SECONDS)
     logging.info("submitted by: %s", runner.params.get("user"))
     logging.info("model_runs:   %s", runner.params["model_runs"])
     logging.info("start_year:   %s", runner.params["start_year"])
@@ -72,23 +65,15 @@ def main(config: Config = Config()):
 def init():
     """Method for calling main."""
     if __name__ == "__main__":
-        exc = None
-        config = Config()
+        # run the model in a try catch block - ensures any exceptions that occur in the
+        # multiprocessing pool are handled and logged correctly.
+        # this prevents the docker container from hanging indefinitely.
         try:
-            # start a timer to kill the container if we reach a timeout
-            t = threading.Timer(config.CONTAINER_TIMEOUT_SECONDS, _exit_container)
-            t.start()
-            # run the model
+            config = Config()
             main(config)
         except Exception as e:
             logging.error("An error occurred: %s", str(e))
-            exc = e
-        finally:
-            # cancel the timer
-            t.cancel()
-            # if there was an exception, raise it
-            if exc is not None:
-                raise exc
+            raise e
 
 
 init()

src/nhp/docker/config.py

@@ -8,8 +8,6 @@
 class Config:
     """Configuration class for Docker container."""
 
-    __DEFAULT_CONTAINER_TIMEOUT_SECONDS = 60 * 60  # 1 hour
-
     def __init__(self):
         """Configuration settings for the Docker container."""
         dotenv.load_dotenv()
@@ -18,8 +16,6 @@ def __init__(self):
         self._data_version = os.environ.get("DATA_VERSION", "dev")
         self._storage_account = os.environ.get("STORAGE_ACCOUNT")
 
-        self._container_timeout_seconds = os.environ.get("CONTAINER_TIMEOUT_SECONDS")
-
     @property
     def APP_VERSION(self) -> str:
         """What is the version of the app?"""
@@ -36,9 +32,3 @@ def STORAGE_ACCOUNT(self) -> str:
         if self._storage_account is None:
             raise ValueError("STORAGE_ACCOUNT environment variable must be set")
         return self._storage_account
-
-    @property
-    def CONTAINER_TIMEOUT_SECONDS(self) -> int:
-        """How long should the container run before timing out?"""
-        t = self._container_timeout_seconds
-        return self.__DEFAULT_CONTAINER_TIMEOUT_SECONDS if t is None else int(t)

tests/unit/nhp/docker/test___main__.py

@@ -1,22 +1,12 @@
 """test docker run."""
 
-import time
 from unittest.mock import Mock, patch
 
 import pytest
 
 from nhp.docker.__main__ import main, parse_args
 
 
-def test_exit_container(mocker):
-    m = mocker.patch("os._exit")
-    import nhp.docker.__main__ as r
-
-    r._exit_container()
-
-    m.assert_called_once_with(2)
-
-
 @pytest.mark.parametrize(
     "args, expected_file, expected_local_storage, expected_save_full_model_results",
     [
@@ -95,7 +85,6 @@ def test_main_azure(mocker):
     config = Mock()
     config.APP_VERSION = "dev"
     config.DATA_VERSION = "dev"
-    config.CONTAINER_TIMEOUT_SECONDS = 3600
     config.STORAGE_ACCOUNT = "sa"
 
     params = {
@@ -127,7 +116,6 @@ def test_main_azure(mocker):
 def test_init(mocker):
     """It should run the main method if __name__ is __main__."""
     config = mocker.patch("nhp.docker.__main__.Config")
-    config().CONTAINER_TIMEOUT_SECONDS = 3600
 
     import nhp.docker.__main__ as r
 
@@ -141,36 +129,6 @@ def test_init(mocker):
         main_mock.assert_called_once_with(config())
 
 
-def test_init_timeout_call_exit(mocker):
-    config = mocker.patch("nhp.docker.__main__.Config")
-    config().CONTAINER_TIMEOUT_SECONDS = 0.1
-
-    import nhp.docker.__main__ as r
-
-    main_mock = mocker.patch("nhp.docker.__main__.main")
-    exit_container_mock = mocker.patch("nhp.docker.__main__._exit_container")
-    main_mock.side_effect = lambda *args, **kwargs: time.sleep(0.2)
-    with patch.object(r, "__name__", "__main__"):
-        r.init()
-
-    exit_container_mock.assert_called_once()
-
-
-def test_init_timeout_dont_call_exit(mocker):
-    import nhp.docker.__main__ as r
-
-    config = mocker.patch("nhp.docker.__main__.Config")
-    config().CONTAINER_TIMEOUT_SECONDS = 0.1
-
-    main_mock = mocker.patch("nhp.docker.__main__.main")
-    exit_container_mock = mocker.patch("nhp.docker.__main__._exit_container")
-    main_mock.side_effect = lambda *args, **kwargs: time.sleep(0.02)
-    with patch.object(r, "__name__", "__main__"):
-        r.init()
-
-    exit_container_mock.assert_not_called()
-
-
 def test_init_catches_exception(mocker):
     # arrange
     mocker.patch("nhp.docker.__main__.main", side_effect=Exception("Test error"))

tests/unit/nhp/docker/test_config.py

@@ -17,7 +17,6 @@ def test_config_sets_values_from_envvars(mocker):
             "APP_VERSION": "app version",
             "DATA_VERSION": "data version",
             "STORAGE_ACCOUNT": "storage account",
-            "CONTAINER_TIMEOUT_SECONDS": "123",
         },
     ):
         config = Config()
@@ -26,7 +25,6 @@ def test_config_sets_values_from_envvars(mocker):
     assert config.APP_VERSION == "app version"
     assert config.DATA_VERSION == "data version"
     assert config.STORAGE_ACCOUNT == "storage account"
-    assert config.CONTAINER_TIMEOUT_SECONDS == 123
 
 
 def test_config_uses_default_values(mocker):
@@ -43,8 +41,6 @@ def test_config_uses_default_values(mocker):
     with pytest.raises(ValueError, match="STORAGE_ACCOUNT environment variable must be set"):
         config.STORAGE_ACCOUNT
 
-    assert config.CONTAINER_TIMEOUT_SECONDS == 3600
-
 
 def test_config_calls_dotenv_load(mocker):
     # arrange

tests/unit/nhp/docker/test_run.py

@@ -63,7 +63,6 @@ def mock_run_with_azure_storage():
     rwas._config.APP_VERSION = "dev"
     rwas._config.DATA_VERSION = "dev"
     rwas._config.STORAGE_ACCOUNT = "sa"
-    rwas._config.CONTAINER_TIMEOUT_SECONDS = 3600
 
     rwas._blob_storage_account_url = "https://sa.blob.core.windows.net"
     rwas._adls_storage_account_url = "https://sa.dfs.core.windows.net"