Paginate all SCIM list requests in the SDK#693
Closed
xinjiezhen-db wants to merge 1 commit intodatabricks:mainfrom
Closed
Paginate all SCIM list requests in the SDK#693xinjiezhen-db wants to merge 1 commit intodatabricks:mainfrom
xinjiezhen-db wants to merge 1 commit intodatabricks:mainfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #693 +/- ##
=======================================
Coverage 15.87% 15.87%
=======================================
Files 94 94
Lines 13773 13773
=======================================
Hits 2186 2186
Misses 11402 11402
Partials 185 185 ☔ View full report in Codecov by Sentry. |
3 tasks
Contributor
|
Continuing with this change in #717 |
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 28, 2023
## Changes This PR sets default parameters for the SCIM pagination APIs, specifically setting StartIndex to 1 (as specified in the [SCIM RFC](https://datatracker.ietf.org/doc/html/rfc7644#section-3.4.2), startindex is 1-based index) and Count to 100 to ensure that we paginate requests to the REST API. This should help reduce load on the SCIM APIs by decreasing the size of response bodies for users of this SDK. This change supersedes #693. ## Tests <!-- How is this tested? Please see the checklist below and also describe any other relevant tests --> - [ ] `make test` passing - [ ] `make fmt` applied - [ ] relevant integration tests applied
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
This PR incorporates two hard-coded changes for the SCIM API in the Python SDK:
startIndex starts at 1 for SCIM APIs, not 0. However, the existing .Pagination.Increment controls both the start index as well as whether the pagination is per-page or per-resource. Later, we should replace this extension with two independent OpenAPI options:
one_indexed(defaulting tofalse) andpagination_basis(defaulting to resource but can be overridden to page).If users don't specify a limit, the SDK will include a hard-coded limit of 100 resources per request. We could add this to the OpenAPI spec as an option
default_limit, which is useful for any non-paginated APIs that later expose pagination options and allow the SDK to gracefully support those. However, we don't want to encourage folks to use this pattern: all new list APIs are required to be paginated from the start.Tests
make testpassingmake fmtapplied