Skip to content

Commit 4260c88

Browse files
committed
chore(tests):
- update tests with daemon context fixture - update mypy config for apscheduler - fix incidental type errors - refresh lock file
1 parent 67b5f40 commit 4260c88

File tree

12 files changed

+104
-45
lines changed

12 files changed

+104
-45
lines changed

packages/cm-models/src/lsst/cmservice/models/db/schedules.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@
1010

1111
from ..enums import ManifestKind
1212
from ..types import KindField
13-
from .campaigns import BaseSQLModel, jsonb_column
13+
from .base import BaseSQLModel
14+
from .campaigns import jsonb_column
1415

1516

1617
class ScheduleBase(BaseSQLModel):
@@ -64,7 +65,6 @@ class ManifestTemplateBase(BaseSQLModel):
6465
kind: KindField = Field(
6566
sa_column=Column("kind", Enum(ManifestKind, length=20, native_enum=False, create_constraint=False)),
6667
)
67-
schedule_id: UUID4 = Field(foreign_key="schedules_v2.id", ondelete="CASCADE")
6868
manifest: dict = jsonb_column("manifest")
6969
metadata_: dict = jsonb_column("metadata", aliases=["metadata", "metadata_"])
7070

@@ -74,12 +74,13 @@ class ManifestTemplate(ManifestTemplateBase, table=True):
7474

7575
model_config = {"validate_assignment": True}
7676
__tablename__: str = "templates_v2" # type: ignore[misc]
77+
schedule_id: UUID4 = Field(foreign_key="schedules_v2.id", ondelete="CASCADE")
7778

7879

7980
class CreateManifestTemplate(ManifestTemplateBase):
8081
"""A validating model for manifest templates in the new schedule API."""
8182

82-
# This model differs from its parent in that the schedule_id is an optional
83-
# field instead of a mandatory FK constraint, allowing it to be used to
83+
# This model differs from its sibling in that the schedule_id is optional
84+
# rather than a mandatory FK constraint, allowing it to be used to
8485
# create new ManifestTemplate objects.
8586
schedule_id: UUID4 | None = None

pyproject.toml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ dynamic = ["version"]
2525
dependencies = [
2626
"alembic==1.16.*",
2727
"anyio==4.10.*",
28+
"apscheduler>=3.11.2,<4",
2829
"asgi-correlation-id>=4.3.4",
2930
"asyncpg==0.30.*",
3031
"click==8.1.*",
@@ -46,15 +47,14 @@ dependencies = [
4647
"structlog==25.*",
4748
"tabulate==0.9.*",
4849
"uvicorn[standard]==0.34.*",
49-
"lsst-cmservice-models",
50-
"apscheduler>=3.11.2",
5150
]
5251

5352
[dependency-groups]
5453
# The LSST dependencies are listed separately to more easily identify the first-
5554
# party dependencies. This dependency group is not optional and will always be
5655
# included in the default installation of the project environment.
5756
lsst = [
57+
"lsst-cmservice-models",
5858
"lsst-ctrl-bps>=29.2025.4700",
5959
"lsst-ctrl-bps-htcondor>=29.2025.4700; sys_platform == 'linux'",
6060
"lsst-ctrl-bps-panda>=29.2025.4700",
@@ -256,7 +256,7 @@ explicit_package_bases = true
256256
namespace_packages = true
257257

258258
[[tool.mypy.overrides]]
259-
module = ["lsst.ctrl.*", "testcontainers.*", "botocore.*", "pandaclient.*", "asyncpg.*"]
259+
module = ["lsst.ctrl.*", "testcontainers.*", "botocore.*", "pandaclient.*", "asyncpg.*", "apscheduler.*"]
260260
ignore_missing_imports = true
261261
disable_error_code = ["import-untyped"]
262262

src/lsst/cmservice/common/daemon_v2.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from collections.abc import Awaitable, Mapping
55
from dataclasses import dataclass, field
66
from datetime import datetime
7+
from types import TracebackType
78
from typing import TYPE_CHECKING, Self, cast
89
from uuid import UUID, uuid5
910

@@ -44,7 +45,9 @@ async def __aenter__(self) -> Self:
4445
self.session = db_session_dependency.sessionmaker()
4546
return self
4647

47-
async def __aexit__(self, exc_type, exc_val, exc_tb) -> None:
48+
async def __aexit__(
49+
self, exc_type: type[BaseException], exc_val: BaseException | None, exc_tb: TracebackType | None
50+
) -> None:
4851
await self.session.close()
4952
return None
5053

@@ -232,7 +235,7 @@ async def consider_nodes(context: DaemonContext) -> None:
232235
await session.commit()
233236

234237

235-
async def do_schedule_stuff(schedule_id: UUID) -> None:
238+
async def daemon_check_schedule(schedule_id: UUID) -> None:
236239
...
237240
breakpoint()
238241
# with daemon context...
@@ -259,7 +262,7 @@ async def consider_schedules(context: DaemonContext) -> None:
259262
context.app.state.scheduler.scheduler.reschedule_job(job_id, job_trigger)
260263
else:
261264
context.app.state.scheduler.scheduler.add_job(
262-
do_schedule_stuff,
265+
daemon_check_schedule,
263266
job_trigger,
264267
id=job_id,
265268
args=[schedule.id],

src/lsst/cmservice/common/templates.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"""
44

55
from collections import deque
6-
from collections.abc import MutableMapping, MutableSequence
6+
from collections.abc import Mapping, MutableMapping, MutableSequence
77
from datetime import datetime, timedelta
88
from typing import Any, Literal
99

@@ -26,7 +26,7 @@ def as_obs_day(value: datetime) -> str:
2626
return f"{value:%Y%m%d}"
2727

2828

29-
def as_exposure(value: datetime, exposure=0) -> str:
29+
def as_exposure(value: datetime, exposure: int = 0) -> str:
3030
return f"{value:%Y%m%d}{exposure:05d}"
3131

3232

@@ -36,12 +36,12 @@ def compile_user_expressions(expressions: MutableMapping) -> dict[str, Any]:
3636
expressed value is cast as a string before being returned as a mapping
3737
of expression name to expression result.
3838
"""
39-
whitelist_modules = {
39+
whitelist_modules: Mapping[str, type] = {
4040
"datetime": datetime,
4141
"timedelta": timedelta,
4242
}
4343
sandbox = ImmutableSandboxedEnvironment()
44-
sandbox.globals = whitelist_modules
44+
sandbox.globals.update(whitelist_modules)
4545

4646
# TODO exception handling here, should the expression blow up
4747
# Compile and evaluate the user expression in the sandbox environment
@@ -60,7 +60,7 @@ async def build_sandbox_and_render_templates(expressions: dict, templates: list[
6060
"""
6161
# . expressions = schedule.expressions
6262
# . templates = schedule.templates
63-
rendered_templates = deque(maxlen=len(templates))
63+
rendered_templates: deque[str] = deque(maxlen=len(templates))
6464
compiled_expressions = compile_user_expressions(expressions)
6565

6666
sandbox = ImmutableSandboxedEnvironment(

src/lsst/cmservice/main.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ async def lifespan(app: FastAPI) -> AsyncGenerator:
6262

6363
# Add Exception Handlers
6464
@app.exception_handler(NoResultFound)
65-
async def notfound_error_handler(request: Request, exc: NoResultFound):
65+
async def notfound_error_handler(request: Request, exc: NoResultFound) -> None:
6666
"""Raise a 404 when the `NoResultFound` exception is raised.
6767
6868
The NoResultFound exception may be raised in a route that uses the
@@ -72,7 +72,7 @@ async def notfound_error_handler(request: Request, exc: NoResultFound):
7272

7373

7474
@app.exception_handler(IntegrityError)
75-
async def duplicate_error_handler(request: Request, exc: IntegrityError):
75+
async def duplicate_error_handler(request: Request, exc: IntegrityError) -> None:
7676
"""Raise a 409 when the `IntegrityError` exception is raised."""
7777
raise HTTPException(status_code=status.HTTP_409_CONFLICT)
7878

src/lsst/cmservice/routers/v2/schedules.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ async def update_schedule_resource(
218218
# TODO consider a custom validators? Might be overkill for this
219219
# The cron string is just a string when it comes back out of the db
220220
schedule_cron = CronStr(schedule.cron)
221-
schedule.next_run_at = schedule_cron.next_run # type: ignore[attr-assigned]
221+
schedule.next_run_at = schedule_cron.next_run # type: ignore
222222

223223
await session.commit()
224224

tests/v2/conftest.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import os
55
from collections.abc import AsyncGenerator, Callable, Generator
66
from typing import TYPE_CHECKING
7+
from unittest.mock import Mock
78
from uuid import NAMESPACE_DNS, uuid4
89

910
import pytest
@@ -12,8 +13,10 @@
1213
from sqlalchemy import insert
1314
from sqlalchemy.pool import NullPool
1415
from sqlalchemy.schema import CreateSchema, DropSchema
16+
from sqlmodel.ext.asyncio.session import AsyncSession
1517
from testcontainers.postgres import PostgresContainer
1618

19+
from lsst.cmservice.common.daemon_v2 import DaemonContext
1720
from lsst.cmservice.common.flags import Features
1821
from lsst.cmservice.config import config
1922
from lsst.cmservice.db.session import DatabaseManager, db_session_dependency
@@ -171,6 +174,17 @@ async def async_client_fixture(
171174
app.dependency_overrides.clear()
172175

173176

177+
@pytest_asyncio.fixture(name="daemon_context", scope="module", loop_scope="module")
178+
async def daemon_context(session: AnyAsyncSession) -> AsyncGenerator[DaemonContext]:
179+
if TYPE_CHECKING:
180+
assert isinstance(session, AsyncSession)
181+
dc = DaemonContext(
182+
app=Mock(),
183+
)
184+
dc.session = session
185+
yield dc
186+
187+
174188
@pytest_asyncio.fixture(scope="function", loop_scope="module")
175189
async def test_campaign(aclient: AsyncClient, manifest_fixtures: None) -> AsyncGenerator[str]:
176190
"""Fixture managing a test campaign with three (additional) nodes, which

tests/v2/test_daemon.py

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,8 @@
66

77
import pytest
88
from sqlmodel import select
9-
from sqlmodel.ext.asyncio.session import AsyncSession
109

11-
from lsst.cmservice.common.daemon_v2 import consider_campaigns, consider_nodes
10+
from lsst.cmservice.common.daemon_v2 import DaemonContext, consider_campaigns, consider_nodes
1211
from lsst.cmservice.common.launchers import LauncherCheckResponse
1312
from lsst.cmservice.models.db.campaigns import Campaign, Node, Task
1413
from lsst.cmservice.models.enums import StatusEnum
@@ -18,7 +17,7 @@
1817

1918

2019
async def test_daemon_campaign(
21-
caplog: pytest.LogCaptureFixture, test_campaign: str, session: AsyncSession
20+
caplog: pytest.LogCaptureFixture, test_campaign: str, daemon_context: DaemonContext
2221
) -> None:
2322
"""Tests the handling of campaigns during daemon iteration, which is
2423
primarily done by checking side effects. This test assesses a test campaign
@@ -27,11 +26,12 @@ async def test_daemon_campaign(
2726
continue to traverse the campaign graph and visit more nodes until the
2827
END node is reached.
2928
"""
29+
session = daemon_context.session
3030

3131
# At first, the test_campaign in a waiting state is not subject to daemon
3232
# consideration
3333
caplog.clear()
34-
await consider_campaigns(session)
34+
await consider_campaigns(daemon_context)
3535

3636
# extract the test campaign id from the fixture url
3737
campaign_id = urlparse(test_campaign).path.split("/")[-2:][0]
@@ -47,7 +47,7 @@ async def test_daemon_campaign(
4747

4848
# now the daemon should consider the running campaign
4949
caplog.clear()
50-
await consider_campaigns(session)
50+
await consider_campaigns(daemon_context)
5151
found_log_messages = 0
5252
for r in caplog.records:
5353
if any(["considering campaign" in r.message, "considering node" in r.message]):
@@ -66,7 +66,7 @@ async def test_daemon_campaign(
6666
await session.commit()
6767

6868
caplog.clear()
69-
await consider_campaigns(session)
69+
await consider_campaigns(daemon_context)
7070
tasks = (await session.exec(select(Task))).all()
7171
# One additional task should be in the table now
7272
assert len(tasks) == 2
@@ -76,7 +76,7 @@ async def test_daemon_campaign(
7676

7777
# The next assessment should produce two nodes to be handled in parallel
7878
caplog.clear()
79-
await consider_campaigns(session)
79+
await consider_campaigns(daemon_context)
8080
tasks = (await session.exec(select(Task))).all()
8181
# Two additional tasks should be in the table now
8282
assert len(tasks) == 4
@@ -89,7 +89,7 @@ async def test_daemon_campaign(
8989

9090
# The next assessment should produce the END node
9191
caplog.clear()
92-
await consider_campaigns(session)
92+
await consider_campaigns(daemon_context)
9393
tasks = (await session.exec(select(Task))).all()
9494
# One additional task should be in the table now
9595
assert len(tasks) == 5
@@ -114,8 +114,9 @@ async def test_daemon_node(
114114
mock_launch: Mock,
115115
caplog: pytest.LogCaptureFixture,
116116
test_campaign: str,
117-
session: AsyncSession,
117+
daemon_context: DaemonContext,
118118
) -> None:
119+
session = daemon_context.session
119120
# set the campaign to running (without involving a campaign machine)
120121
campaign_id = urlparse(url=test_campaign).path.split("/")[-2:][0]
121122
campaign = await session.get_one(Campaign, campaign_id)
@@ -124,8 +125,8 @@ async def test_daemon_node(
124125

125126
# after the equivalent of a single iteration, the test campaign's START
126127
# Node will be on the task list with a transition from waiting->ready.
127-
await consider_campaigns(session)
128-
await consider_nodes(session)
128+
await consider_campaigns(daemon_context)
129+
await consider_nodes(daemon_context)
129130

130131
# As we continue to iterate the daemon over the campaign's 5 nodes
131132
# (including its START and END), each node in the graph is evolved.
@@ -139,8 +140,8 @@ async def test_daemon_node(
139140
i = 12
140141
while end_node.status is not StatusEnum.accepted:
141142
i -= 1
142-
await consider_campaigns(session)
143-
await consider_nodes(session)
143+
await consider_campaigns(daemon_context)
144+
await consider_nodes(daemon_context)
144145
# the end node is expunged from the session as a side effect when the
145146
# graph is built
146147
session.add(end_node)

tests/v2/test_rpc.py

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,8 @@
77
import pytest
88
from httpx import AsyncClient, Response
99
from sqlmodel import select
10-
from sqlmodel.ext.asyncio.session import AsyncSession
1110

12-
from lsst.cmservice.common.daemon_v2 import consider_nodes
11+
from lsst.cmservice.common.daemon_v2 import DaemonContext, consider_nodes
1312
from lsst.cmservice.common.launchers import LauncherCheckResponse
1413
from lsst.cmservice.models.db.campaigns import Task
1514

@@ -28,11 +27,12 @@ async def test_rpc_process_node(
2827
caplog: pytest.LogCaptureFixture,
2928
test_campaign: str,
3029
aclient: AsyncClient,
31-
session: AsyncSession,
30+
daemon_context: DaemonContext,
3231
) -> None:
3332
"""Tests the manual step-through of a campaign node using the "process" RPC
3433
API.
3534
"""
35+
session = daemon_context.session
3636
campaign_id = urlparse(url=test_campaign).path.split("/")[-2:][0]
3737
node_id = uuid5(UUID(campaign_id), "START.1")
3838

@@ -51,7 +51,7 @@ async def process_once() -> Response:
5151

5252
# now the daemon should consider the prepared campaign
5353
caplog.clear()
54-
await consider_nodes(session)
54+
await consider_nodes(daemon_context)
5555
found_log_messages = 0
5656
for r in caplog.records:
5757
if any(["evolving node" in r.message]):
@@ -70,7 +70,7 @@ async def process_once() -> Response:
7070
# Use the RPC API to process the node into a "running" state
7171
x = await process_once()
7272
assert x.is_success
73-
await consider_nodes(session)
73+
await consider_nodes(daemon_context)
7474

7575
x = await aclient.get(f"/v2/nodes/{node_id}")
7676
assert x.is_success
@@ -79,7 +79,7 @@ async def process_once() -> Response:
7979
# Use the RPC API to process the node into a "accepted" state
8080
x = await process_once()
8181
assert x.is_success
82-
await consider_nodes(session)
82+
await consider_nodes(daemon_context)
8383

8484
x = await aclient.get(f"/v2/nodes/{node_id}")
8585
assert x.is_success
@@ -122,7 +122,7 @@ async def test_rpc_with_breakpoint_node(
122122
mock_launch: Mock,
123123
test_campaign: str,
124124
aclient: AsyncClient,
125-
session: AsyncSession,
125+
daemon_context: DaemonContext,
126126
) -> None:
127127
"""Tests the addition of a Breakpoint node in a campaign, and that process-
128128
ing the campaign does not proceed past the breakpoint until it is applied
@@ -167,15 +167,15 @@ async def process_once() -> Response:
167167
for _ in range(5):
168168
x = await process_once()
169169
assert x.is_success
170-
await consider_nodes(session)
170+
await consider_nodes(daemon_context)
171171

172172
# confirm that the breakpoint node does not advance from "running"
173173
r = await aclient.get(breakpoint_url)
174174
assert r.json()["status"] == "running"
175175

176176
x = await process_once()
177177
assert x.is_success
178-
await consider_nodes(session)
178+
await consider_nodes(daemon_context)
179179

180180
r = await aclient.get(breakpoint_url)
181181
assert r.json()["status"] == "running"
@@ -195,4 +195,4 @@ async def process_once() -> Response:
195195
# confirm that the rpc proceeds to the next node
196196
x = await process_once()
197197
assert x.is_success
198-
await consider_nodes(session)
198+
await consider_nodes(daemon_context)

0 commit comments

Comments
 (0)