airbytehq / airbytehq/airbyte-python-cdk
`DayClampingStrategy` ceiling skips a full day for already-midnight values (`clamping: DAY` loses one day per window)
- Vorherrschende Sprache
- Python
- Sterne
- 26
- Forks
- 53
- Ø Merge
- 2 T. 6 Std.
- Gemergte PRs (30 T.)
- 10
Beschreibung
## Summary
`DayClampingStrategy.clamp` with `is_ceiling=True` (the default wired for `clamping: {target: DAY}` on a `DatetimeBasedCursor`) unconditionally adds one day after flooring the value to midnight. A value that is ALREADY midnight-aligned therefore jumps a full day forward. When the concurrent cursor uses the clamped value as the next slice's lower boundary, every stepped window loses one day of data.
## Root cause
[`airbyte_cdk/sources/streams/concurrent/clamping.py` L34-L42 (v7.23.8)](https://github.com/airbytehq/airbyte-python-cdk/blob/v7.23.8/airbyte_cdk/sources/streams/concurrent/clamping.py#L34-L42):
```python
class DayClampingStrategy(ClampingStrategy):
def clamp(self, value: datetime) -> datetime:
return_value = value.replace(hour=0, minute=0, second=0, microsecond=0)
if self._is_ceiling:
return return_value + timedelta(days=1)
return return_value
```
There is no already-aligned guard. Compare with `MonthClampingStrategy` in the same file ([L45-L56](https://github.com/airbytehq/airbyte-python-cdk/blob/v7.23.8/airbyte_cdk/sources/streams/concurrent/clamping.py#L45-L56)), which checks `needs_to_round = value.day != 1` and returns the value untouched when it is already on the boundary. `WeekClampingStrategy` has the equivalent guard as well. Only the DAY strategy is missing it.
## Reproduction
```python
from datetime import datetime, timezone
from airbyte_cdk.sources.streams.concurrent.clamping import DayClampingStrategy
s = DayClampingStrategy() # is_ceiling=True, as built for clamping: {target: DAY}
print(s.clamp(datetime(2024, 1, 1, tzinfo=timezone.utc)))
# 2024-01-02 00:00:00+00:00 - a midnight value is pushed a full day forward
```
Slice-level effect, measured with the real concurrent cursor (start 2024-01-01, end 2024-06-15, step P30D, granularity PT1S, clamping DAY):
```
2024-01-02 .. 2024-01-31T23:59:59 <- 2024-01-01 lost
2024-02-02 .. 2024-03-02T23:59:59 <- gap: 2024-02-01 never queried
2024-03-04 .. 2024-04-02T23:59:59 <- gap: 2024-03-03 never queried
2024-04-04 .. 2024-05-03T23:59:59 <- repeats every window
```
The slice loop sets the next lower boundary from the clamped upper and re-ceils it, so the skipped day recurs at every window boundary.
## Impact
Any `DatetimeBasedCursor` configured with `clamping: {target: DAY}` and a stepped range silently drops one calendar day of data per window. `WEEK`/`MONTH` targets are unaffected thanks to their guards.
## Suggested fix
Mirror the `MonthClampingStrategy` guard: return the value unchanged when it is already midnight-aligned, e.g. `needs_to_round = (value.hour, value.minute, value.second, value.microsecond) != (0, 0, 0, 0)`.
## Precedent
Found while reviewing https://github.com/airbytehq/airbyte/pull/75495 (source-klaviyo reporting streams): day-aligned report windows were the natural fix there, `clamping: {target: DAY}` was the obvious tool, and pre-merge verification caught the dropped day - the connector shipped with a midnight-floored start date instead.
Beitragsleitfaden
Rechercherichtung
Start in airbyte_cdk/sources/streams/concurrent/clamping.py at DayClampingStrategy.clamp and compare its ceiling behavior with MonthClampingStrategy and WeekClampingStrategy. Run the reported midnight reproduction and verify that an already midnight-aligned value no longer advances by a day while non-aligned ceiling behavior and stepped cursor windows remain correct.
Vom Indexierungsmodell aus dem Issue-Text verfasst.
Bewertung
- Tech-Stack
- python
- Bereich
- backend
- Issue-Typ
- Bug
- Schwierigkeit
- 2/5
- Geschätzter Aufwand
- 1-3 Stunden
- Aktivitätsstatus
- Aktiv
- Klarheit
- Klar beschrieben
- Anfängerfreundlichkeit
- 85/100