-
Notifications
You must be signed in to change notification settings - Fork 24
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
Release/32 #64
Merged
Merged
Release/32 #64
Changes from all commits
Commits
Show all changes
52 commits
Select commit
Hold shift + click to select a range
ff52377
Added Refactored Tap Boilerplate Classes
b738db8
ECDDC-476
efb3ea0
Implemented some logic, need to rethink inheritance, especially initi…
33459da
Pushing more changes, will test after this
72e3caa
Intermediary commit
906b3cc
Added last_account_appraisal_date to loan_accounts.json
30cbdfe
Implemented bookmark resume and ABC for Generators, with Loan Account…
c451325
Commit before fixing Appraisal Order Bug
284f56a
Finished implementation of Loan Accounts Generator/Processor
0039dd6
Small Bugfixes
6d82085
Merge branch 'master' into release/32
2c8fca6
Merge branch 'release/32' into feature/ECDCC-476_Extract-loans
19800f3
Small fixes suggested by Alex via PR Comments
f8617c0
Solved __iter__ method and __next__ method duplicated code according …
f7c4d20
Added a comment so we don't forget about the deduplication_key in the…
21c8ef2
Merge branch 'feature/ECDCC-476_Extract-loans' into 'release/32'
6868cbd
Also added record_count
2a58d92
Merge branch 'feature/ECDCC-476_Extract-loans' into 'release/32'
7bc95df
adjusted the last_datetime var from sync_endpoint func to use the loo…
DownstreamDataTeam 3eb4038
Merge branch 'feature/ECDCC-500_lookback_window_bugfix' into 'release…
062e19d
Revert "Merge branch 'feature/ECDCC-500_lookback_window_bugfix' into …
fa82719
Added Refactored Tap Boilerplate Classes
7b11dd8
ECDDC-476
952efdc
Implemented some logic, need to rethink inheritance, especially initi…
12d3628
Pushing more changes, will test after this
b7ee002
Intermediary commit
4d106f4
Added last_account_appraisal_date to loan_accounts.json
c4de88f
Implemented bookmark resume and ABC for Generators, with Loan Account…
139a7a7
Commit before fixing Appraisal Order Bug
8531705
Finished implementation of Loan Accounts Generator/Processor
3c39c6f
Small Bugfixes
80ab4f6
Small fixes suggested by Alex via PR Comments
c9afcbe
Solved __iter__ method and __next__ method duplicated code according …
0bbfd3a
Added a comment so we don't forget about the deduplication_key in the…
2a5e45b
Also added record_count
532e55b
adjusted the last_datetime var from sync_endpoint func to use the loo…
DownstreamDataTeam e7e3653
Revert "Merge branch 'feature/ECDCC-500_lookback_window_bugfix' into …
6f04d6d
Merge remote-tracking branch 'origin/release/32' into release/32
DownstreamDataTeam b4dec65
Started the loan_repayments refactor
DownstreamDataTeam a9a4717
Finished the refactor
DownstreamDataTeam 7762921
Fixed missing deduplication_key error
DownstreamDataTeam cd8d18a
Fixed up loan repayments refactor
96475ee
Added gitlab-ci config
20632f3
Revert "Added gitlab-ci config"
4d835f7
Merge branch 'feature/ECDDC-519_loan_repayments_refactor' into 'relea…
6a923a4
Bugfix for records ignored when filtering by modified_date on records…
a70b1e1
Merge branch 'bugfix/ECDDC-559-release-32-fix-loan-accounts-appraisal…
b50a8d4
Fixed processor to take bookmark into account correctly, fixed tap-te…
936a08a
Also added replication key to tests
e3f4a05
Adjusted bookmark tests to accept multiple replication keys
cba648e
Merge branch 'bugfix/ECDDC-559-release-32-fix-loan-accounts-appraisal…
1e78c8b
Merge branch 'master' into release/32
DownstreamDataTeam File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 akey1 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.