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 get BigQuery datasets in over 50 datasets environment #5667

Merged
merged 2 commits into from
Dec 15, 2021

Conversation

katsugeneration
Copy link
Contributor

What type of PR is this? (check all applicable)

  • Bug Fix

Description

This implementation fixes getting BigQuery datasets for get_schema operation in over about 50 datasets environment.

BigQuery dataset list API supports pagination. But old implementation do not use it. It causes user to get only several datasets returned by an api call and not to get more than datasets.

New implementation use pagination for fixing it.

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

@susodapop
Copy link
Contributor

Thanks for this contribution! I'm happy to merge, but I don't have an environment to test against BigQuery. Can you advise whether this is a drop-in replacement for the previous implementation? Will it break existing data sources?

@susodapop susodapop self-assigned this Dec 13, 2021
@katsugeneration
Copy link
Contributor Author

Thanks for review.

this is a drop-in replacement for the previous implementation?

Yes. This implementation only use same api and return dataset information in the same format for the previous implementation.

Will it break existing data sources?

No. This implementation only change the number of datasets for get_schema operation results by getting additional datasets if use have datasets over than 50.
So, if user will change to this implementation and will get schema first time, redash will behave in the same as being added datasets to BigQuery by user.

Copy link
Contributor

@susodapop susodapop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@susodapop
Copy link
Contributor

Thanks for an awesome contribution. This is huge for BQ users 👍

@susodapop susodapop merged commit 6458650 into getredash:master Dec 15, 2021
harveyrendell pushed a commit to pushpay/redash that referenced this pull request Jan 8, 2025
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.

2 participants