Skip to content

Commit a85ba2d

Browse files
fix: address CodeRabbit review feedback
- Add 'Z' suffix to timestamp isoformat in get_count() and get_timeline() - Move shutil import to module level in tests - Use assertEqual instead of assertLessEqual for exact count verification
1 parent b18c3a3 commit a85ba2d

2 files changed

Lines changed: 11 additions & 10 deletions

File tree

structured_logging.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,8 @@ def count(
419419
params.append(feature_id)
420420
if since:
421421
conditions.append("timestamp >= ?")
422-
params.append(since.isoformat())
422+
ts = since.isoformat().replace("+00:00", "Z")
423+
params.append(ts if ts.endswith("Z") else since.isoformat())
423424

424425
where_clause = " AND ".join(conditions) if conditions else "1=1"
425426
cursor.execute(f"SELECT COUNT(*) FROM logs WHERE {where_clause}", params)
@@ -460,7 +461,12 @@ def get_timeline(
460461
GROUP BY bucket, agent_id
461462
ORDER BY bucket
462463
""",
463-
(bucket_minutes, bucket_minutes, since.isoformat(), until.isoformat()),
464+
(
465+
bucket_minutes,
466+
bucket_minutes,
467+
since.isoformat().replace("+00:00", "Z"),
468+
until.isoformat().replace("+00:00", "Z"),
469+
),
464470
)
465471

466472
rows = cursor.fetchall()

test_structured_logging.py

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
"""
77

88
import json
9+
import shutil
910
import sqlite3
1011
import tempfile
1112
import threading
@@ -81,7 +82,6 @@ def setUp(self):
8182

8283
def tearDown(self):
8384
"""Clean up temporary files."""
84-
import shutil
8585
shutil.rmtree(self.temp_dir, ignore_errors=True)
8686

8787
def test_creates_database(self):
@@ -120,7 +120,6 @@ def setUp(self):
120120

121121
def tearDown(self):
122122
"""Clean up temporary files."""
123-
import shutil
124123
shutil.rmtree(self.temp_dir, ignore_errors=True)
125124

126125
def test_creates_logs_directory(self):
@@ -212,7 +211,6 @@ def setUp(self):
212211

213212
def tearDown(self):
214213
"""Clean up temporary files."""
215-
import shutil
216214
shutil.rmtree(self.temp_dir, ignore_errors=True)
217215

218216
def test_query_by_level(self):
@@ -302,7 +300,6 @@ def setUp(self):
302300

303301
def tearDown(self):
304302
"""Clean up temporary files."""
305-
import shutil
306303
shutil.rmtree(self.temp_dir, ignore_errors=True)
307304

308305
def test_export_json(self):
@@ -360,7 +357,6 @@ def setUp(self):
360357

361358
def tearDown(self):
362359
"""Clean up temporary files."""
363-
import shutil
364360
shutil.rmtree(self.temp_dir, ignore_errors=True)
365361

366362
def test_concurrent_writes(self):
@@ -434,7 +430,6 @@ def setUp(self):
434430

435431
def tearDown(self):
436432
"""Clean up temporary files."""
437-
import shutil
438433
shutil.rmtree(self.temp_dir, ignore_errors=True)
439434

440435
def test_cleanup_old_entries(self):
@@ -462,8 +457,8 @@ def test_cleanup_old_entries(self):
462457
count = cursor.fetchone()[0]
463458
conn.close()
464459

465-
# Should have at most max_entries
466-
self.assertLessEqual(count, 10)
460+
# Should have exactly max_entries after cleanup
461+
self.assertEqual(count, 10)
467462

468463

469464
if __name__ == "__main__":

0 commit comments

Comments
 (0)