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

Fix landings pagination #48

Merged
merged 3 commits into from Oct 29, 2021
Merged

Fix landings pagination #48

merged 3 commits into from Oct 29, 2021

Conversation

loeakaodas
Copy link
Contributor

@loeakaodas loeakaodas commented Oct 15, 2021

Description of change

Added pagination to the landings stream as per TDL-15821

Should close: #47, #25, and #28

Manual QA steps

  • ran tap manually
  • ran against singer-check-tap and target-stitch --dry-run
  • CircleCI tests passing

Risks

Rollback steps

  • revert this branch

@loeakaodas
Copy link
Contributor Author

@KrisPersonal here's the code coverage for this tap
htmlcov.zip

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'), [])
Copy link
Contributor

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?

Copy link
Contributor Author

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

@LindsayOllie
Copy link

Hello - is there anything I need to do on my end in order to see these changes in my database?

@leslievandemark leslievandemark merged commit b52decc into master Oct 29, 2021
@leslievandemark leslievandemark deleted the fix-landings-pagination branch October 29, 2021 13:38
hanyufoodles added a commit to hanyufoodles/tap-typeform that referenced this pull request Dec 23, 2021
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.

Typeform hidden fields results are not populating or appending to referrer URL
4 participants