-
Notifications
You must be signed in to change notification settings - Fork 386
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
Use token-based pagination for Jobs List operations #2810
Conversation
As we couldn't easily switch `databricks_job` resource to use Go SDK, we were still using offset-based pagination that has a significant load onto the control plane. This PR changes list operation to use recommended token-based approach until we switch to Go SDK. This fixes #2807
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.
LGTM with minor nits
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.
Implementation seems good to me. Can we extend TestJobsData to make a second request with the page token?
Jobs []Job `json:"jobs"` | ||
HasMore bool `json:"has_more,omitempty"` | ||
NextPageToken string `json:"next_page_token,omitempty"` | ||
PrevPageToken string `json:"prev_page_token,omitempty"` |
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.
Just to confirm, you're adding PrevPageToken for completeness, even though we don't use it in this PR.
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.
Yes, only for completeness...
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.
LGTM! Missed the addition of the next_page_token
Changes
As we couldn't easily switch
databricks_job
resource to use Go SDK, we were still using offset-based pagination that has a significant load onto the control plane. This PR changes list operation to use recommended token-based approach until we switch to Go SDK.This fixes #2807
Tests
make test
run locallyrelevant change indocs/
folderinternal/acceptance