SAT: add threshold_days incremental test option
#12715
Conversation
| def compare_cursor_with_threshold(record_value, state_value, threshold_days) -> bool: | ||
| """ | ||
| Checks if the record's cursor value is older or equal to the state cursor value. | ||
|
|
||
| If the threshold_days option is set, the values will be converted to dates so that the time-based offset can be applied. | ||
| :raises dateutil.parser._parser.ParserError: if threshold_days is passed with non-date cursor values. | ||
| """ | ||
| if threshold_days: | ||
| record_date_value = record_value if isinstance(record_value, datetime) else dateutil.parser.parse(record_value) | ||
| state_date_value = state_value if isinstance(state_value, datetime) else dateutil.parser.parse(state_value) | ||
|
|
||
| return record_date_value >= (state_date_value - timedelta(days=threshold_days)) | ||
|
|
||
| return record_value >= state_value |
There was a problem hiding this comment.
This is only considering the threshold_days option to be valid for date-based cursors. An offset may be desired for other types of cursors (maybe ints?), but I couldn't think of a good use case and it would make the parsing and comparison more difficult.
github-actions
bot
added
the
area/documentation
There was a problem hiding this comment.
looks good to me! But have you tested on different connectors that send dates in different formats or just a plain integers for epoch time? Off the top of my head, I believe Hubspot sends some of their times as integers so probably worth confirming that dateutil.parse() still works with that as well.
...e-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_incremental.py
Show resolved
Hide resolved
...e-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_incremental.py
Outdated
Show resolved
Hide resolved
...e-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_incremental.py
Outdated
Show resolved
Hide resolved
@brianjlai Good call-out! Hubspot itself does some converting to return ISO strings, but there are some other connectors that do return unix timestamps. Added support for that in 9cf16cc
@Phlair From what I've seen the only connectors affected are google ads (#12668) and google analytics (#12662), which do have issues created. |
|
/test connector=bases/source-acceptance-test
|
|
/publish connector=bases/source-acceptance-test
|
* SAT: add `threshold_days` incremental test option * fix: support cursor values that are already dates * dont use constant value * update docs * use pendulum for date parsing * bump cdk version * use pendulum for duration * add support for unix timestamps * bump version, update changelog
What
Support running incremental source acceptance tests for sources that use a lookback window to include records older than the requested state cursor.
close #12467
How
threshold_daysoption for incremental tests to control the amount of days to allow before the state cursordateutil.parser.parse, aParserErrorwill be raised.This has been confirmed to allow enabling the incremental tests for the Google Analytics connector.
Recommended reading order
source_acceptance_test/config.pysource_acceptance_test/test_incremental.pyunit_tests/test_incremental.pyTests
Unit