Remove most_recent_record arg from Cursor.close_slice

[ambirdsall]

After #35843 removed the most_recent_record argument from the logic of the close_slice method, this cleans up the Cursor interface to not include the argument at all. Relates to https://github.com/airbytehq/airbyte-internal-issues/issues/6314

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 18, 2024 11:39pm

[natikgadzhi]

* What went wrong:
Execution failed for task ':flakeCheck'.

Oh my gawd. The amount of time we stumble at this. I can't wait to switch the CDK to Poetry and switch it to Ruff for linting and formatting with pre-commit hook so we don't have to waste time and CI credits on stuff like this. /cc @erohmensing

[natikgadzhi]

Honestly weirdly I have no idea what's going on here. In the previous PR we introduced most_recent_record that is being observed, but here I see we're just going back to sticking with the slice. How does this work?

[ambirdsall]

Honestly weirdly I have no idea what's going on here. In the previous PR we introduced most_recent_record that is being observed, but here I see we're just going back to sticking with the slice. How does this work?

@natikgadzhi a quick overview of how this logic has evolved:

1. The old logic: the cursor state was calculated in Cursor.close_slice by finding the most recent value out of the most recent record read, the slice's end time, and the previous state.
2. The new logic (last PR): a new Cursor.observe method was added to track the highest cursor value observed within its slice boundaries and the logic in Cursor.close_slice was simplified: now it only compares that highest observed cursor value with the previous state. This does a better job handling reads where the cursor values are sorted in descending order (since the most recently-read cursor value passed to Cursor.close_slice would be the lowest, not the highest) or when new values are added between the latest read and its old slice boundary (e.g. a connector that has a DatetimeBasedCursor with one-month granularity is synced on the 20th of the month and then new records are added on the 25th). The PR shipping this logic (Base datetime cursor state off latest observed record #35843) kept the old method signature to limit the blast radius of the new change.
3. the cleanup (this PR): since the most recently-read record at the time close_slice is called is no longer used to calculate the cursor state, it is removed from the method call altogether. It's a fairly trivial change, and perhaps could have been included in the last PR—especially since any connectors with custom cursor overrides pin the CDK to specific versions anyway.
4. update custom cursors (next PR): the Greenhouse and Chargebee sources have to be updated to handle Cursor.observe calls correctly (i.e. track cursor values there instead of Cursor.close_slice) and to respect the new close_slice signature. Because the connector CI pipeline needs a published CDK version to validate changes, those updates cannot get a green build until the current PR has to be merged and published.

[girarda]

lgtm! thanks for the quick follow up!