-
Notifications
You must be signed in to change notification settings - Fork 50
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
fix(Workable): use append write disposition for candidate subresources #366
fix(Workable): use append write disposition for candidate subresources #366
Conversation
) | ||
yield candidates_resource | dlt.transformer( | ||
name=f"candidates_{sub_endpoint}", write_disposition="merge" | ||
name=f"candidates_{sub_endpoint}", write_disposition="append" |
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.
hey! you are right, we missed primary_key (or merge_key) here, candidates details response has no id key to make it primary. I think it makes more sense to set write_disposition='replace', to avoid data duplication.
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.
@AstrakhantsevaAA given that candidates is incremental, it will replace all existing data with just updated ones.
So for example,
- Initial sync syncs 100 candidates and 10 offers
- Candidate 20 now has an offer
- Second pull will replace candidate offers with only a single offer for candidate 20
While merge may have duplicates, it's better than the alternative of replacing everything
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.
right!
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.
Thank you for your contribution!
) | ||
yield candidates_resource | dlt.transformer( | ||
name=f"candidates_{sub_endpoint}", write_disposition="merge" | ||
name=f"candidates_{sub_endpoint}", write_disposition="append" |
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.
right!
@ethanve can you rebase on master pls? |
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.
LGTM!
Tell us what you do here
Given that there's no primary key set for candidate sub-resources, we should just use
append
verified source
)Relevant issue
issue #
More PR info