Skip to content

Commit

Permalink
Refactor start.configure_logging for module path
Browse files Browse the repository at this point in the history
#3

The `start.configure_logging` method was previously configured to accept
a file path to a Python logging configuration file. This works well for
Docker, because the path is predictable inside the Docker image, but
limits usefulness of logging_conf.py with `pip install inboard`.

This commit will refactor `configure_logging` to accept a module path,
in the style of `set_app_module`. References to `configure_logging`,
pytest fixtures in conftest.py, and unit tests in test_start.py will be
updated accordingly. The README will describe changes to `LOGGING_CONF`.

This commit will also clarify some testing function argument names.

- `start.set_conf_path`: `module` -> `module_stem`
- `tmp_dir` -> `tmp_path` when the `tmp_path` fixture is used directly
  • Loading branch information
br3ndonland committed Sep 2, 2020
1 parent 3dc8085 commit ff9155a
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 152 deletions.
1 change: 0 additions & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
"BASIC_AUTH_PASSWORD": "plunge-germane-tribal-pillar",
"LOG_FORMAT": "uvicorn",
"LOG_LEVEL": "debug",
"LOGGING_CONF": "logging_conf.py",
"PROCESS_MANAGER": "uvicorn",
"WITH_RELOAD": "true"
},
Expand Down
10 changes: 5 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,9 @@ ENV APP_MODULE="package.custom.module:api" WORKERS_PER_CORE="2"
- `"/app/inboard/gunicorn_conf.py"` (the default file provided with the Docker image)
- Custom:
- `GUNICORN_CONF="/app/package/custom_gunicorn_conf.py"`
- Notes
- Feel free to use the [`gunicorn_conf.py`](./inboard/gunicorn_conf.py) from this repo as a starting point for your own custom configuration.
- `GUNICORN_CONF` accepts a file path, instead of a module path, because Gunicorn is typically only run from within Docker, where the file path will be predictable.
- `HOST`: Host IP address (inside of the container) where Gunicorn will listen for requests.
- Default: `"0.0.0.0"`
- Custom: _TODO_
Expand Down Expand Up @@ -299,11 +301,9 @@ ENV APP_MODULE="package.custom.module:api" WORKERS_PER_CORE="2"

### Logging

- `LOGGING_CONF`: Path to a [Python logging configuration file](https://docs.python.org/3/library/logging.config.html). The configuration must be a new-style `.py` file, containing a configuration dictionary object named `LOGGING_CONFIG`. The `LOGGING_CONFIG` dictionary will be passed to [`logging.config.dictConfig()`](https://docs.python.org/3/library/logging.config.html). See [br3ndonland/inboard#3](https://github.com/br3ndonland/inboard/pull/3) for more details on this design choice.
- Default:
- `"/app/inboard/logging_conf.py"` (the default file provided with the Docker image)
- Custom:
- `LOGGING_CONF="/app/package/custom_logging.py"`
- `LOGGING_CONF`: Python module containing a logging [configuration dictionary object](https://docs.python.org/3/library/logging.config.html) named `LOGGING_CONFIG`. The `LOGGING_CONFIG` dictionary will be passed to [`logging.config.dictConfig()`](https://docs.python.org/3/library/logging.config.html). See [br3ndonland/inboard#3](https://github.com/br3ndonland/inboard/pull/3) for more details on this design choice.
- Default: `"inboard.logging_conf"` (the default module provided with inboard)
- Custom: For a logging config module at `/app/package/custom_logging.py`, `LOGGING_CONF="package.custom_logging"`.
- `LOG_COLORS`: Whether or not to color log messages. Currently only supported for `LOG_FORMAT="uvicorn"`.
- Default:
- Auto-detected based on [`sys.stdout.isatty()`](https://docs.python.org/3/library/sys.html#sys.stdout).
Expand Down
4 changes: 1 addition & 3 deletions inboard/gunicorn_conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,7 @@

# Gunicorn config variables
try:
logconfig_dict = configure_logging(
logging_conf=os.getenv("LOGGING_CONF", "/app/inboard/logging_conf.py")
)
logconfig_dict = configure_logging()
except Exception as e:
if use_loglevel == "debug":
msg = "Error loading logging config with Gunicorn:"
Expand Down
55 changes: 24 additions & 31 deletions inboard/start.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,45 +11,39 @@
import uvicorn # type: ignore


def set_conf_path(module: str) -> str:
def set_conf_path(module_stem: str) -> str:
"""Set the path to a configuration file."""
conf_var = str(os.getenv(f"{module.upper()}_CONF"))
conf_var = str(os.getenv(f"{module_stem.upper()}_CONF"))
if Path(conf_var).is_file():
conf_path = conf_var
elif Path(f"/app/inboard/{module}_conf.py").is_file():
conf_path = f"/app/inboard/{module}_conf.py"
elif Path(f"/app/inboard/{module_stem}_conf.py").is_file():
conf_path = f"/app/inboard/{module_stem}_conf.py"
else:
conf_path = f"/{module}_conf.py"
os.environ[f"{module.upper()}_CONF"] = conf_path
conf_path = f"/{module_stem}_conf.py"
os.environ[f"{module_stem.upper()}_CONF"] = conf_path
return conf_path


def configure_logging(
logger: Logger = logging.getLogger(),
logging_conf: str = str(os.getenv("LOGGING_CONF", "/app/inboard/logging_conf.py")),
) -> Dict[str, Any]:
"""Configure Python logging based on a path to a logging configuration file."""
def configure_logging(logger: Logger = logging.getLogger()) -> Dict[str, Any]:
"""Configure Python logging based on a path to a logging module or file."""
try:
logging_conf_path = Path(logging_conf)
if logging_conf_path.is_file() and logging_conf_path.suffix == ".py":
spec = importlib.util.spec_from_file_location("confspec", logging_conf_path)
logging_conf_module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(logging_conf_module) # type: ignore
if hasattr(logging_conf_module, "LOGGING_CONFIG"):
logging_conf_dict = getattr(logging_conf_module, "LOGGING_CONFIG")
else:
raise AttributeError(f"No LOGGING_CONFIG in {logging_conf_module}.")
if isinstance(logging_conf_dict, dict):
logging.config.dictConfig(logging_conf_dict)
message = f"Logging dict config loaded from {logging_conf}."
logger.debug(message)
return logging_conf_dict
else:
raise TypeError("LOGGING_CONFIG is not a dictionary instance.")
logging_conf_path = str(os.getenv("LOGGING_CONF", "inboard.logging_conf"))
spec = importlib.util.find_spec(logging_conf_path)
logging_conf_module = importlib.util.module_from_spec(spec) # type: ignore
spec.loader.exec_module(logging_conf_module) # type: ignore
if hasattr(logging_conf_module, "LOGGING_CONFIG"):
logging_conf_dict = getattr(logging_conf_module, "LOGGING_CONFIG")
else:
raise AttributeError(f"No LOGGING_CONFIG in {logging_conf_module}.")
if isinstance(logging_conf_dict, dict):
logging.config.dictConfig(logging_conf_dict)
message = f"Logging dict config loaded from {logging_conf_path}."
logger.debug(message)
return logging_conf_dict
else:
raise ImportError("Valid path to .py logging config file required.")
raise TypeError("LOGGING_CONFIG is not a dictionary instance.")
except Exception as e:
message = f"Error when configuring logging: {e}."
message = f"Error when setting logging module: {e}."
logger.debug(message)
raise

Expand Down Expand Up @@ -125,8 +119,7 @@ def start_server(

if __name__ == "__main__":
logger = logging.getLogger()
logging_conf_path = set_conf_path("logging")
logging_conf_dict = configure_logging(logger=logger, logging_conf=logging_conf_path)
logging_conf_dict = configure_logging(logger=logger)
gunicorn_conf_path = set_conf_path("gunicorn")
app_module = set_app_module(logger=logger)
run_pre_start_script(logger=logger)
Expand Down
58 changes: 24 additions & 34 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@


@pytest.fixture(scope="session")
def app_module_tmp_dir(tmp_path_factory: TempPathFactory) -> Path:
def app_module_tmp_path(tmp_path_factory: TempPathFactory) -> Path:
"""Copy app modules to temporary directory to test custom app module paths."""
tmp_dir = tmp_path_factory.mktemp("app")
shutil.copytree(f"{Path(pre_start_module.__file__).parent}", f"{tmp_dir}/tmp_app")
Expand Down Expand Up @@ -68,50 +68,40 @@ def logging_conf_dict(mocker: MockerFixture) -> Dict[str, Any]:


@pytest.fixture
def logging_conf_path(monkeypatch: MonkeyPatch) -> Path:
"""Set path to default logging configuration file."""
path = Path(logging_conf_module.__file__)
monkeypatch.setenv("LOGGING_CONF", str(path))
assert os.getenv("LOGGING_CONF") == str(path)
def logging_conf_module_path(monkeypatch: MonkeyPatch) -> str:
"""Set module path to logging_conf.py."""
path = "inboard.logging_conf"
monkeypatch.setenv("LOGGING_CONF", path)
assert os.getenv("LOGGING_CONF") == path
return path


@pytest.fixture
def logging_conf_path_tmp(tmp_path: Path) -> Path:
"""Copy logging configuration file to custom temporary file."""
tmp_file = shutil.copy(Path(logging_conf_module.__file__), tmp_path)
return Path(tmp_file)


@pytest.fixture
def logging_conf_path_tmp_txt(tmp_path: Path) -> Path:
"""Create custom temporary logging config file with incorrect extension."""
tmp_file = tmp_path / "tmp_logging_conf.txt"
return Path(tmp_file)
@pytest.fixture(scope="session")
def logging_conf_tmp_path(tmp_path_factory: TempPathFactory) -> Path:
"""Copy logging configuration module to custom temporary location."""
tmp_dir = tmp_path_factory.mktemp("tmp_log")
shutil.copy(Path(logging_conf_module.__file__), f"{tmp_dir}/tmp_log.py")
return tmp_dir


@pytest.fixture
def logging_conf_path_tmp_no_dict(tmp_path: Path) -> Path:
"""Create custom temporary logging config file.
- Correct extension
- No `LOGGING_CONFIG` object
"""
tmp_file = tmp_path / "tmp_logging_conf.py"
@pytest.fixture(scope="session")
def logging_conf_tmp_path_no_dict(tmp_path_factory: TempPathFactory) -> Path:
"""Create temporary logging config file without logging config dict."""
tmp_dir = tmp_path_factory.mktemp("tmp_log_no_dict")
tmp_file = tmp_dir / "no_dict.py"
with open(Path(tmp_file), "x") as f:
f.write("print('Hello, World!')\n")
return Path(tmp_file)
return tmp_dir


@pytest.fixture
def logging_conf_path_tmp_incorrect_type(tmp_path: Path) -> Path:
"""Create custom temporary logging config file.
- Correct extension
- Incorrect data type for `LOGGING_CONFIG` object
"""
tmp_file = tmp_path / "tmp_logging_conf_incorrect_type.py"
@pytest.fixture(scope="session")
def logging_conf_tmp_path_incorrect_type(tmp_path_factory: TempPathFactory) -> Path:
"""Create temporary logging config file with incorrect LOGGING_CONFIG type."""
tmp_dir = tmp_path_factory.mktemp("tmp_log_incorrect_type")
tmp_file = tmp_dir / "incorrect_type.py"
with open(Path(tmp_file), "x") as f:
f.write("LOGGING_CONFIG: list = ['Hello', 'World']\n")
return Path(tmp_file)
return tmp_dir


@pytest.fixture
Expand Down
Loading

0 comments on commit ff9155a

Please sign in to comment.