Skip to content

Commit

Permalink
feat(config): Refactor config module
Browse files Browse the repository at this point in the history
- Use nested models for configs
- Promote hardcoded values to config variables
  • Loading branch information
tcjennings committed Dec 17, 2024
1 parent 2a59dff commit 1d027fc
Show file tree
Hide file tree
Showing 43 changed files with 399 additions and 256 deletions.
16 changes: 8 additions & 8 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,14 @@ target-version = "py311"

[tool.ruff.lint]
ignore = [
"COM812",
"N802",
"N803",
"N806",
"N812",
"N813",
"N815",
"N816",
"COM812", # missing-trailing-comma
"N802", # invalid-function-name
"N803", # invalid-argument-name
"N806", # non-lowercase-variable-in-function
"N812", # lowercase-imported-as-non-lowercase
"N813", # camelcase-imported-as-constant
"N815", # mixed-case-variable-in-class-scope
"N816", # mixed-case-variable-in-global-scope
]
select = [
"E", # pycodestyle
Expand Down
4 changes: 2 additions & 2 deletions src/lsst/cmservice/cli/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ def server() -> None:
@run_with_asyncio
async def init(*, reset: bool) -> None: # pragma: no cover
"""Initialize the service database."""
logger = structlog.get_logger(config.logger_name)
engine = create_database_engine(config.database_url, config.database_password)
logger = structlog.get_logger(__name__)
engine = create_database_engine(config.db.url, config.db.password)
await initialize_database(engine, logger, schema=db.Base.metadata, reset=reset)
await engine.dispose()

Expand Down
21 changes: 11 additions & 10 deletions src/lsst/cmservice/common/htcondor.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import subprocess
from typing import Any

from ..config import config
from .enums import StatusEnum
from .errors import CMHTCondorCheckError, CMHTCondorSubmitError

Expand Down Expand Up @@ -49,9 +50,9 @@ def write_htcondor_script(
should_transfer_files="Yes",
when_to_transfer_output="ON_EXIT",
get_env=True,
request_cpus=1,
request_memory="512M",
request_disk="1G",
request_cpus=config.htcondor.request_cpus,
request_memory=config.htcondor.request_mem,
request_disk=config.htcondor.request_disk,
)
options.update(**kwargs)

Expand Down Expand Up @@ -86,16 +87,16 @@ def submit_htcondor_job(
try:
with subprocess.Popen(
[
"condor_submit",
config.htcondor.condor_submit_bin,
htcondor_script_path,
],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
) as sbatch: # pragma: no cover
sbatch.wait()
if sbatch.returncode != 0:
assert sbatch.stderr
msg = sbatch.stderr.read().decode()
) as condor_submit: # pragma: no cover
condor_submit.wait()
if condor_submit.returncode != 0:
assert condor_submit.stderr
msg = condor_submit.stderr.read().decode()
raise CMHTCondorSubmitError(f"Bad htcondor submit: {msg}")
except Exception as e:
raise CMHTCondorSubmitError(f"Bad htcondor submit: {e}") from e
Expand Down Expand Up @@ -126,7 +127,7 @@ def check_htcondor_job(
if htcondor_id is None: # pragma: no cover
raise CMHTCondorCheckError("No htcondor_id")
with subprocess.Popen(
["condor_q", "-userlog", htcondor_id, "-af", "JobStatus", "ExitCode"],
[config.htcondor.condor_q_bin, "-userlog", htcondor_id, "-af", "JobStatus", "ExitCode"],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
) as condor_q: # pragma: no cover
Expand Down
11 changes: 6 additions & 5 deletions src/lsst/cmservice/common/slurm.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import subprocess

from ..config import config
from .enums import StatusEnum
from .errors import CMSlurmCheckError, CMSlurmSubmitError

Expand Down Expand Up @@ -62,15 +63,15 @@ def submit_slurm_job(
try:
with subprocess.Popen(
[
"sbatch",
config.slurm.sbatch_bin,
"-o",
log_url,
"--mem",
"16448",
config.slurm.memory,
"--account",
"rubin:production",
config.slurm.account,
"-p",
"milano",
config.slurm.partition,
"--parsable",
script_url,
],
Expand Down Expand Up @@ -114,7 +115,7 @@ def check_slurm_job(
return StatusEnum.running
try:
with subprocess.Popen(
["sacct", "--parsable", "-b", "-j", slurm_id], stdout=subprocess.PIPE
[config.slurm.sacct_bin, "--parsable", "-b", "-j", slurm_id], stdout=subprocess.PIPE
) as sacct: # pragma: no cover
sacct.wait()
if sacct.returncode != 0:
Expand Down
185 changes: 155 additions & 30 deletions src/lsst/cmservice/config.py
Original file line number Diff line number Diff line change
@@ -1,60 +1,185 @@
from pydantic import Field
from pydantic_settings import BaseSettings
from safir.logging import LogLevel, Profile
from pydantic import BaseModel, Field
from pydantic_settings import BaseSettings, SettingsConfigDict

__all__ = ["Configuration", "config"]


class Configuration(BaseSettings):
"""Configuration for cm-service."""
model_config = SettingsConfigDict(
env_nested_delimiter="__",
nested_model_default_partial_update=True,
case_sensitive=False,
extra="ignore",
)
"""A common settings config for each BaseSettings model."""


class HTCondorConfiguration(BaseModel):
"""Configuration settings for htcondor client operations.
Set via HTCONDOR__FIELD environment variables.
"""

condor_submit_bin: str = Field(
description="Name of condor_submit client binary",
default="condor_submit",
)

condor_q_bin: str = Field(
description="Name of condor_q client binary",
default="condor_q",
)

request_cpus: int = Field(
description="Number of cores to request when submitting an htcondor job.",
default=1,
)

request_mem: str = Field(
description="Amount of memory requested when submitting an htcondor job.",
default="512M",
)

request_disk: str = Field(
description="Amount of disk space requested when submitting an htcondor job.",
default="1G",
)


class SlurmConfiguration(BaseModel):
"""Configuration settings for slurm client operations.
Set via SLURM__FIELD environment variables.
Note
----
Default SBATCH_* variables could work just as well, but it is useful to
have this as a document of what settings are actually used.
"""

sacct_bin: str = Field(
description="Name of sacct slurm client binary",
default="sacct",
)

sbatch_bin: str = Field(
description="Name of sbatch slurm client binary",
default="sbatch",
)

memory: str = Field(
description="Amount of memory requested when submitting a slurm job.",
default="16448",
)

account: str = Field(
description="Account used when submitting a slurm job.",
default="rubin:production",
)

partition: str = Field(
description="Partition requested when submitting a slurm job.",
default="milano",
)


class AsgiConfiguration(BaseModel):
"""Configuration for the application's ASGI web server."""

title: str = Field(
description="Title of the ASGI application",
default="cm-service",
)

host: str = Field(
description="The host address to which the asgi server should bind",
default="0.0.0.0",
)

port: int = Field(
description="Port number for the asgi server to listen on",
default=8080,
)

prefix: str = Field(
description="The URL prefix for the cm-service API",
default="/cm-service/v1",
title="The URL prefix for the cm-service API",
validation_alias="CM_URL_PREFIX",
)

database_url: str = Field(
frontend_prefix: str = Field(
description="The URL prefix for the frontend web app",
default="/web_app",
)


class LoggingConfiguration(BaseModel):
"""Configuration for the application's logging facility."""

level: str = Field(
default="INFO",
title="Log level of the application's logger",
)

profile: str = Field(
default="development",
title="Application logging profile",
)


class DatabaseConfiguration(BaseSettings):
"""Database configuration nested model.
Set according to the validation_alias for each field.
Note
----
Validation aliases in nested BaseModels do not work for environment
variables, so this model uses BaseSettings with a model_config instead of
BaseModel.
"""

model_config = model_config

url: str = Field(
default="",
title="The URL for the cm-service database",
description="The URL for the cm-service database",
validation_alias="CM_DATABASE_URL",
)

database_password: str | None = Field(
password: str | None = Field(
default=None,
title="The password for the cm-service database",
description="The password for the cm-service database",
validation_alias="CM_DATABASE_PASSWORD",
)

database_schema: str | None = Field(
schema: str | None = Field(
default=None,
title="Schema to use for cm-service database",
description="Schema to use for cm-service database",
validation_alias="CM_DATABASE_SCHEMA",
)

database_echo: bool = Field(
echo: bool = Field(
default=False,
title="SQLAlchemy engine echo setting for the cm-service database",
description="SQLAlchemy engine echo setting for the cm-service database",
validation_alias="CM_DATABASE_ECHO",
)

profile: Profile = Field(
default=Profile.development,
title="Application logging profile",
validation_alias="CM_LOG_PROFILE",
)

logger_name: str = Field(
default="cmservice",
title="The root name of the application's logger",
validation_alias="CM_LOGGER",
)
class Configuration(BaseSettings):
"""Configuration for cm-service.
log_level: LogLevel = Field(
default=LogLevel.INFO,
title="Log level of the application's logger",
validation_alias="CM_LOG_LEVEL",
)
Nested models may be consumed from environment variables named according to
the pattern 'NESTED_MODEL__FIELD' or via any `validation_alias` applied to
a field.
"""

model_config = model_config

# Nested Models
asgi: AsgiConfiguration = AsgiConfiguration()
db: DatabaseConfiguration = DatabaseConfiguration()
htcondor: HTCondorConfiguration = HTCondorConfiguration()
logging: LoggingConfiguration = LoggingConfiguration()
slurm: SlurmConfiguration = SlurmConfiguration()


config = Configuration()
Expand Down
4 changes: 2 additions & 2 deletions src/lsst/cmservice/daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@


async def main_loop() -> None:
logger = structlog.get_logger(config.logger_name)
engine = create_database_engine(config.database_url, config.database_password)
logger = structlog.get_logger(__name__)
engine = create_database_engine(config.db.url, config.db.password)

async with engine.begin():
session = await create_async_session(engine, logger)
Expand Down
2 changes: 1 addition & 1 deletion src/lsst/cmservice/db/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@
class Base(DeclarativeBase):
"""Base class for DB tables"""

metadata = MetaData(schema=config.database_schema)
metadata = MetaData(schema=config.db.schema)
3 changes: 1 addition & 2 deletions src/lsst/cmservice/db/row.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@
CMMissingFullnameError,
CMMissingIDError,
)
from ..config import config

logger = get_logger(config.logger_name)
logger = get_logger(__name__)

T = TypeVar("T", bound="RowMixin")

Expand Down
Loading

0 comments on commit 1d027fc

Please sign in to comment.