updated prospects stream sync function #42
Conversation
|
Please enhance the PR description by including details about the specific data discrepancies or scenarios that prompted the fix. This will provide better clarity on the problem being addressed and the solution proposed by the PR. |
| command: | | ||
| python3 -mvenv /usr/local/share/virtualenvs/tap-pardot | ||
| source /usr/local/share/virtualenvs/tap-pardot/bin/activate | ||
| pip install -U 'pip==22.2.2' 'setuptools==65.3.0' |
There was a problem hiding this comment.
Should we consider using the pip version 23.3.2?
| def sync(self): | ||
| self.pre_sync() | ||
| for rec in self.sync_page(): | ||
| yield rec | ||
| self.post_sync() |
There was a problem hiding this comment.
Since the pre_sync and post_sync functions are not overridden in the parent class, they seem unnecessary here. Would it be beneficial to remove them from the class definition?
Correct me here if I am wrong.
There was a problem hiding this comment.
pre_sync and post_sync are not implemented/modified in this class, but do exist in the parent class, and can be modified to add/remove functionality.
i am not removing it to keep this pattern constant across all streams of the tap.
There was a problem hiding this comment.
@sgandhi1311 please re-review
This reverts commit 29ff131.
* revert prospects fix #42
Description of change
ref: https://developer.salesforce.com/docs/marketing/pardot/guide/prospects-v4.html#parameters-to-specify-which-results-are-returned
The default implementation would refer to the last record on the current page to get the next
updated_atfor the next request, this resulted in few records being missed in the iteration.Manual QA steps
Risks
Rollback steps