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

Hotfix: Changed deposit_accounts sorting field #91

Merged
merged 343 commits into from
Sep 12, 2022

Conversation

DownstreamDataTeam
Copy link
Contributor

Description of change

Changed the sorting criteria from id to lastModifiedDate because of how the MultithreadedBookmarkGenerator works: changes the filter at every n records.
If the sort is on id, there's a high chance that a record with the lastModifiedDate close to present to be in that batch, thus the extraction continues from that date and misses older records that were not extracted.

Manual QA steps

Risks

Rollback steps

  • revert this branch

Alexandru Rosca and others added 30 commits March 23, 2022 14:41
…ease/40'

[ECDDC-653] Refactored main.py and unit tests

See merge request mambucom/product/ecosystem/mambu-marketplace/connectors/singer/tap-mambu!72
…_breakdown_stream

# Conflicts:
#	tap_mambu/tap_mambu_refactor/main.py
…to 'release/40'

[ECDDC-646] New Sonar config version

See merge request mambucom/product/ecosystem/mambu-marketplace/connectors/singer/tap-mambu!65
[ECDDC-591] Fixed issue with users deduplication key

See merge request mambucom/product/ecosystem/mambu-marketplace/connectors/singer/tap-mambu!78
…' into 'release/40'

[ECDDC-603] Added interest accrual breakdown stream

See merge request mambucom/product/ecosystem/mambu-marketplace/connectors/singer/tap-mambu!74
…se/40'

[ECDDC-649] Added task_link_key field to tasks stream

See merge request mambucom/product/ecosystem/mambu-marketplace/connectors/singer/tap-mambu!75
[ECDDC-652] Adjusted Snyk dev test

See merge request mambucom/product/ecosystem/mambu-marketplace/connectors/singer/tap-mambu!67
…ease/40'

[ECDDC-653] Finished implementing catalog automatic fields checker test

See merge request mambucom/product/ecosystem/mambu-marketplace/connectors/singer/tap-mambu!77
…b.com:mambucom/product/ecosystem/mambu-marketplace/connectors/singer/tap-mambu into feature/ECDDC-657_Refactor-audit-trail-stream
Alexandru Rosca and others added 21 commits June 30, 2022 23:37
…nto 'release/45'

[ECDDC-729] Refactored offset and bookmark multithreaded generators

See merge request mambucom/product/ecosystem/mambu-marketplace/connectors/singer/tap-mambu!120
… 'release/45'

[ECDDC-716] Deposit accounts missing records

See merge request mambucom/product/ecosystem/mambu-marketplace/connectors/singer/tap-mambu!123
… to one using a custom dict class "HashableDict" which implements a hash and unique keys for each dict (generated from the json dump data)
[ECDDC-726] Resolve issues in the PR

See merge request mambucom/product/ecosystem/mambu-marketplace/connectors/singer/tap-mambu!131
[ECDDC-726] Replaced all json.dumps with tuple conversions, as they are faster to compute a hash for

See merge request mambucom/product/ecosystem/mambu-marketplace/connectors/singer/tap-mambu!132
…'release/45'

[ECDDC-749] Added python_requires to setup.py

See merge request mambucom/product/ecosystem/mambu-marketplace/connectors/singer/tap-mambu!136
…e/45'

[ECDDC-752] Fixed deposit_accounts missing records

See merge request mambucom/product/ecosystem/mambu-marketplace/connectors/singer/tap-mambu!138
@singer-bot
Copy link

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.

Copy link
Contributor

@bryantgray bryantgray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. There are also other generators that have the endpoint_sorting_criteria set to id. I see LoanTransactionsGenerator, InterestAccrualBreakdownGenerator , DepositTransactionsGenerator, GlJournalEntriesGenerator, and LoanAccountsGenerator. Is it possible they could have the same issues in the future since they also sort by id?

@DownstreamDataTeam
Copy link
Contributor Author

This looks good. There are also other generators that have the endpoint_sorting_criteria set to id. I see LoanTransactionsGenerator, InterestAccrualBreakdownGenerator , DepositTransactionsGenerator, GlJournalEntriesGenerator, and LoanAccountsGenerator. Is it possible they could have the same issues in the future since they also sort by id?

They shouldn't cause an issue, because the filter criteria is set on creationDate (or something similar), which should be in sync with the id, so the sorting should be the same for both.

@bryantgray bryantgray merged commit 3ed0335 into singer-io:master Sep 12, 2022
@bryantgray bryantgray mentioned this pull request Sep 12, 2022
@singer-bot
Copy link

You did it @DownstreamDataTeam!

Thank you for signing the Singer Contribution License Agreement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants