-
Notifications
You must be signed in to change notification settings - Fork 19
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
Add pagination visitors stream #129
Conversation
tap_pendo/streams.py
Outdated
if self.name in ["visitors"]: | ||
# If number of records equal to record then assume there are more records to be synced | ||
# and save the last filter value. Otherwise assume we have extracted all records | ||
loop_for_records = self.get_record_count() >= self.record_limit |
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.
I strongly suggest not following this approach as it creates inverted dependancy, this function belongs to the generic implementation of the stream class, and this condition refers to the name of a child class.
i would suggest that you modify this method in the Visitors
class below
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.
or you can convert this into a generic implementation
I have added visitors test data, that will be available for testing tomorrow. Please expect change at base.py#L34 to reduce the daily execution time in CCi.
|
Description of change
Added the the custom pagination support for the Visitors stream TDL-24201
Now visitor records will be synced in the batches of
record_limit
which defaults to 100,000 instead of syncing them all in one go reducing the memory stress.Manual QA steps
include_anonymous_visitors="true"
visitors
and child streamvisitor_history
streams are selectedRisks
Rollback steps