[ISSUE] Users.List and other listing API using iterators do not work correctly when using Count

[tcesnik-veza]

Description
All the listing API that uses iterators do not allow listing of all the items, but they merely fetch just one page based on the Count parameter.
They all contain this code:

iterator := listing.NewIterator(
	&request,
	getNextPage,
	getItems,
	nil)

The problem is that the last parameter in the NewIterator constructor is not set, so new requests cannot be created based on the responses.

This makes it impossible to use the intended pattern of this API for iterating:

for iterator.HasNext(ctx) {
	object, err := iterator.Next(ctx)
	...
}

Reproduction

pageSizeListUsers := 2
client := databricks.Must(databricks.NewWorkspaceClient())
iterator := client.Users.List(ctx, iam.ListUsersRequest{
	Count:      pageSizeListUsers,
})
for iterator.HasNext(ctx) {
	user, _ := iterator.Next(ctx)
	fmt.Println(user)
}

With e.g. 10 users configured on the workspace, this code only prints 2 - which is the page size configured.

Expected behavior
The code should print all users configured on the workspace.

Is it a regression?
No - tested on the last version 0.25.0.

Debug Logs
/

Other Information
/

Additional context
/

[tcesnik-veza]

After some more research, I can conclude that the issue #641 has the same problem, as ListAll is actually relying on the same iterator behavior when Count is used.

[mgyucht]

Thanks for raising this. For my understanding, what is your reasoning for using Count? We are actually considering removing all pagination-based parameters from request structures to prevent this kind of confusion. For SCIM in particular, we have special logic to make sure that a default page size is set when listing users/groups/service principals, so you don't need to configure this field yourself.

[mgyucht]

In any case, after looking at this, I think you're right, we need to change the SCIM API definition to make this work properly.

[tcesnik-veza]

Thanks for raising this. For my understanding, what is your reasoning for using Count? We are actually considering removing all pagination-based parameters from request structures to prevent this kind of confusion. For SCIM in particular, we have special logic to make sure that a default page size is set when listing users/groups/service principals, so you don't need to configure this field yourself.

The main reason was basically to support rare cases with huge amounts of users, to not overload the network message size and possibly even client memory.

[nfx]

@tcesnik-veza we're thinking of completely removing count parameter from the high-level interface, so that issues like this one don't cause confusion - see the following PR:

• Improve semantics for listing methods #569

The main reason was basically to support rare cases with huge amounts of users, to not overload the network message size and possibly even client memory.

default page size for SCIM is 100, meaning at most 100 users per page. How would it overload the network?..

[tcesnik-veza]

I am fine with changing the high-level interface to not include count parameter, if pagination is used behind the scenes.

[nfx]

@tcesnik-veza re: large workspaces/accounts and optimization - see the discussion here - databrickslabs/ucx#638

[mgyucht]

@tcesnik-veza this has been fixed in today's release of the Go SDK: default page size is 100 users/groups/service principals when listing. Please try it out and let us know if you have any feedback; otherwise, I'll close this out.

[tcesnik-veza]

Looks good, although haven't extensively tested it.