Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[low-code connectors] lookback window should be applied on the cursor instead of on the start date #15628

Closed
1 task
Tracked by #19658
girarda opened this issue Aug 12, 2022 · 2 comments
Closed
1 task
Tracked by #19658
Assignees
Labels
team/extensibility type/bug Something isn't working

Comments

@girarda
Copy link
Contributor

girarda commented Aug 12, 2022

Current Behavior

The DatetimeStreamSlicer's lookback window is only applied on the start date, which means it won't work for subsequent syncs.

context: #15027 (comment)

Expected Behavior

The lookback window should be based off the cursor date instead of the state date when performing a sync that uses stream slicing. If the look back window is 3 days, on the first time sync, records starting from 3 days before the start date should be retrieved. For the subsequent incremental sync, the we should attempt to get records starting from 3 days before the date specified in the incoming state json.

Implementation Approach:

Within the datetime_stream_slicer.py DatetimeStreamSlicer.stream_slices() method, we derive what the starting time of the request should be. We apply the lookback to the start time specified and then choose the greater of the start_datetime and cursor_datetime. We should actually compare the two datetimes first and then apply the lookback accordingly.

Acceptance Criteria

  • When running an incremental sync with state, it should yield records starting after the cursor date subtracted from the look back window

Good context for validation:
A good integration to test this on is source_wikipedia_pageviews because it doesn't require authentication and has datetime stream slicing implemented already. It doesn't have a lookback window specified, but you can add that in for testing purposes.

When running the first time sync, you should see the lookback window reflected by the date of the records received when you run a read command. Now you can pass in a state json starting later than the start_datetime using --state. On this second, you should see see records with dates earlier than that of the date in state.json according to the lookback window defined.

@brianjlai
Copy link
Contributor

make the acceptance criteria one bullet point and keep the context

maxi297 added a commit that referenced this issue Dec 6, 2022
maxi297 added a commit that referenced this issue Dec 8, 2022
#20156)

* [ISSUE #15628] apply lookback window on earliest datetime between start and cursor

* [ISSUE #15628] update release information and clean return statement
@maxi297
Copy link
Contributor

maxi297 commented Dec 8, 2022

PR associated with this change: #20156

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team/extensibility type/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants