elastic / elastic/ai-github-actions
[framework-best-practices] Parse workflow log date filters with datetime
- Dominant language
- Python
- Stars
- 11
- Forks
- 16
- Avg merge
- 22h 9m
- Merged PRs (30d)
- 31
Description
## Framework / Library Best Practices Findings
### 1. Workflow log date filters compare ISO timestamps as strings
**Library:** Python stdlib `datetime` (`requires-python >=3.10` in `pyproject.toml`)
**Library feature:** `datetime.fromisoformat()` with timezone-aware comparisons
**Current code:** `scripts/fetch-workflow-logs.py:11-12` documents `--since` and `--until` as ISO 8601 filters, but `scripts/fetch-workflow-logs.py:40-48` normalizes date-only `--until` by appending a string suffix and `scripts/fetch-workflow-logs.py:71-80` compares `run["created_at"]` to user-provided filter values with raw string ordering:
```python
def _is_before_since_boundary(run: dict, since: str | None) -> bool:
if since is None:
return False
return run.get("created_at", "") < since
def _is_after_until_boundary(run: dict, until: str | None) -> bool:
if until is None:
return False
return run.get("created_at", "") > until
```
**Simplification:** Parse GitHub timestamps and user filter values into timezone-aware `datetime` objects once, normalize `Z` to `+00:00`, and compare datetimes instead of lexicographic strings. Date-only `--until` can be represented as the end of that UTC day with `datetime.combine(date, time.max, tzinfo=timezone.utc)`.
**Why this matters:** The CLI accepts ISO 8601 inputs, which can legally include timezone offsets. Lexicographic ordering does not preserve chronological ordering across equivalent instants with different offsets. For example, a run at `2025-01-01T00:00:00Z` is after `2025-01-01T01:00:00+02:00` chronologically, but string comparison treats the `Z` timestamp as less because `"00" < "01"`, so the script can stop paging early or exclude the wrong runs.
**Documentation:** (docs.python.org/redacted)
## Suggested Actions
- [ ] Convert `--since`, normalized `--until`, and each run `created_at` value to timezone-aware `datetime` objects before filtering.
- [ ] Add regression coverage for offset-bearing ISO inputs so equivalent UTC instants are included/excluded correctly.
---
[What is this?](https://ela.st/github-ai-tools) | [From workflow: Trigger Framework Best Practices](https://github.com/elastic/ai-github-actions/actions/runs/28946939416)
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
Contributor guide
Research direction
Read scripts/fetch-workflow-logs.py at lines 11-12, 40-48, and 71-80, starting with how the CLI parses --since and --until and how workflow timestamps are filtered. Add regression coverage for timezone-offset inputs; done means equivalent instants are compared chronologically and date-only --until still covers the full UTC day.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- cli
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100