-
Notifications
You must be signed in to change notification settings - Fork 20
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 landings pagination #48
Conversation
@KrisPersonal here's the code coverage for this tap |
tap_typeform/streams.py
Outdated
while page_count > 1: | ||
response = get_landings(atx, form_id, token, next_page=True) | ||
page_count = response.get('page_count', 1) | ||
data.extend(response.get('items'), []) |
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 think this is the wrong approach. We could hit a memory limit by holding on to every page with this extend
.
Is there an advantage to this over writing a page of records at a time?
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.
That is valid, I've moved the pagination to get_landings()
and it now yields one page at a time
Hello - is there anything I need to do on my end in order to see these changes in my database? |
This reverts commit b52decc.
Description of change
Added pagination to the
landings
stream as per TDL-15821Should close: #47, #25, and #28
Manual QA steps
singer-check-tap
andtarget-stitch --dry-run
Risks
Rollback steps