Release/32

[DownstreamDataTeam]

Description of change

When extracting loan accounts using both the Last Modified Date filter and Account Appraisal Date filter, records with appraisal date after the bookmark are skipped if their Last Modified Date is older than the bookmark.

Manual QA steps

• Ran before & after using a set start date (and tested also with bookmark) and compared the results, which included new records after the change

Risks

Rollback steps

• revert this branch

[cmerrick]

Hi @DownstreamDataTeam, thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes.

[dmosorast]

@DownstreamDataTeam I ran through the tap-tester test suite and it looks like the start date and bookmarks tests are now failing with a value difference.

======================================================================
FAIL: test_run (test_bookmarks.BookmarksTest) (stream='loan_accounts')
Verify that we can get multiple pages of data for each stream
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/code/tap-mambu/tests/test_bookmarks.py", line 92, in test_run
    self.assertEqual(first_sync_bookmark_value,
AssertionError: '2021-06-08T07:55:28.000000Z' != '2022-02-02T08:11:38.000000Z'
- 2021-06-08T07:55:28.000000Z
?    ^  ^  ^  ^ ^^ ^
+ 2022-02-02T08:11:38.000000Z
?    ^  ^  ^  ^ ^^ ^


======================================================================
FAIL: test_run (test_start_date.StartDateTest) (stream='loan_accounts')
Verify that running a sync, then moving the start date into the
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/code/tap-mambu/tests/test_start_date.py", line 146, in test_run
    self.assertGreaterEqual(
AssertionError: datetime.datetime(2019, 8, 26, 14, 23, 44, tzinfo=<UTC>) not greater than or equal to datetime.datetime(2021, 1, 1, 0, 0, tzinfo=<UTC>)

----------------------------------------------------------------------
Ran 6 tests in 488.085s

FAILED (failures=2)

It seems like this might be related to both fields being involved in the bookmarking strategy now, but in any case, the test failures should be addressed before I feel confident merging this. Thank you!

[dmosorast]

This is sort of an intereting case, since I don't think we've ever used two fields for a replication key before. This use case is a sort of key1 OR key2 approach, rather than a key1 AND key2 approach (like would be the case for a composite primary key).

The replication key metadata in a SaaS tap is mostly informational, so that may not be an issue, but it's an interesting piece that I hadn't considered. What folks have done in the past where the max of two fields is the replication key would be to consolidate them into a single manufactured field (like row_updated or something) and bookmark based on that, but this approach seems more fine grained as far as state goes, so I'm leaning towards it being good as-is. I just wanted to raise that as a consideration on my end.

Tests passed locally, so I'm good to release this now.

[singer-bot]

You did it @DownstreamDataTeam!

Thank you for signing the Singer Contribution License Agreement.