Skip to content

Commit 49f23bc

Browse files
authored
fix MonitorExecution.started bug (#5864)
1 parent 650f70d commit 49f23bc

File tree

5 files changed

+111
-2
lines changed

5 files changed

+111
-2
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ Changes can also be flagged with a GitHub label for tracking purposes. The URL o
4040
### Fixed
4141
- Fixed pagination bugs on some tables [#5819](https://github.com/ethyca/fides/pull/5819)
4242
- Fixed load_samples to wrap variables in quotes to prevent YAML parsing errors [#5857](https://github.com/ethyca/fides/pull/5857)
43+
- Fixed incorrect value being set for `MonitorExecution.started` column [#5864](https://github.com/ethyca/fides/pull/5864)
4344

4445
## [2.56.2](https://github.com/ethyca/fides/compare/2.56.1...2.56.2)
4546

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
"""fix monitorexecution.started default
2+
3+
Revision ID: 1152c1717849
4+
Revises: 3c58001ad310
5+
Create Date: 2025-03-10 16:04:57.104689
6+
7+
"""
8+
9+
import sqlalchemy as sa
10+
from alembic import op
11+
from sqlalchemy.dialects import postgresql
12+
13+
# revision identifiers, used by Alembic.
14+
revision = "1152c1717849"
15+
down_revision = "3c58001ad310"
16+
branch_labels = None
17+
depends_on = None
18+
19+
20+
def upgrade():
21+
# Set the new server default
22+
# And set the datetime to be timezone aware
23+
op.alter_column(
24+
"monitorexecution",
25+
"started",
26+
existing_type=sa.DateTime(),
27+
type_=sa.DateTime(timezone=True),
28+
server_default=sa.text("now()"),
29+
)
30+
31+
# Set the datetime to be timezone aware
32+
op.alter_column(
33+
"monitorexecution",
34+
"completed",
35+
existing_type=sa.DateTime(),
36+
type_=sa.DateTime(timezone=True),
37+
)
38+
39+
40+
def downgrade():
41+
# Remove the server default
42+
# And set the datetime to not be timezone aware
43+
op.alter_column(
44+
"monitorexecution",
45+
"started",
46+
server_default=None,
47+
existing_type=sa.DateTime(timezone=True),
48+
type_=sa.DateTime(),
49+
)
50+
51+
# Set the datetime to not be timezone aware
52+
op.alter_column(
53+
"monitorexecution",
54+
"completed",
55+
existing_type=sa.DateTime(timezone=True),
56+
type_=sa.DateTime(),
57+
)

src/fides/api/db/base.py

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from fides.api.db.base_class import Base
55
from fides.api.models.application_config import ApplicationConfig
66
from fides.api.models.asset import Asset
7+
from fides.api.models.attachment import Attachment, AttachmentReference
78
from fides.api.models.audit_log import AuditLog
89
from fides.api.models.authentication_request import AuthenticationRequest
910
from fides.api.models.client import ClientDetail

src/fides/api/models/detection_discovery.py

+4-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from __future__ import annotations
22

3-
from datetime import datetime, timezone
3+
from datetime import datetime
44
from enum import Enum
55
from typing import Any, Dict, Iterable, List, Optional, Type
66

@@ -388,7 +388,9 @@ class MonitorExecution(Base):
388388
)
389389
status = Column(String, nullable=True)
390390
started = Column(
391-
DateTime(timezone=True), nullable=True, default=datetime.now(timezone.utc)
391+
DateTime(timezone=True),
392+
nullable=True,
393+
server_default=func.now(),
392394
)
393395
completed = Column(DateTime(timezone=True), nullable=True)
394396
classification_instances = Column(

tests/ops/models/test_detection_discovery.py

+48
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from fides.api.models.detection_discovery import (
1111
DiffStatus,
1212
MonitorConfig,
13+
MonitorExecution,
1314
MonitorFrequency,
1415
StagedResource,
1516
fetch_staged_resources_by_type_query,
@@ -633,3 +634,50 @@ def test_update_monitor_config_execution_trigger_logic(
633634
)
634635
assert mc.execution_start_date == expected_date
635636
db.delete(mc)
637+
638+
639+
class TestMonitorExecutionModel:
640+
"""Tests for the MonitorExecution model"""
641+
642+
@pytest.fixture
643+
def monitor_config_key(self, db: Session, monitor_config) -> str:
644+
"""Returns a monitor config key for testing"""
645+
return monitor_config.key
646+
647+
def test_started_timestamp_is_set_on_creation(
648+
self, db: Session, monitor_config_key
649+
) -> None:
650+
"""Test that the started timestamp is set correctly when creating a new record"""
651+
# Create first record
652+
first_execution = MonitorExecution.create(
653+
db=db,
654+
data={
655+
"monitor_config_key": monitor_config_key,
656+
"status": "running",
657+
},
658+
)
659+
660+
# Small delay to ensure timestamps would be different
661+
import time
662+
663+
time.sleep(0.1)
664+
665+
# Create second record
666+
second_execution = MonitorExecution.create(
667+
db=db,
668+
data={
669+
"monitor_config_key": monitor_config_key,
670+
"status": "running",
671+
},
672+
)
673+
674+
# Verify timestamps are different (not a constant default)
675+
assert first_execution.started != second_execution.started
676+
677+
# Verify timestamps are recent
678+
now = datetime.now(timezone.utc)
679+
assert (now - first_execution.started).total_seconds() < 5
680+
assert (now - second_execution.started).total_seconds() < 5
681+
682+
# Verify second timestamp is later than first
683+
assert second_execution.started > first_execution.started

0 commit comments

Comments
 (0)