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.

Sign up for GitHub

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

Jump to bottom

Use default page size for jumping to desired offset #38457

Merged
merged 2 commits into from
Jun 22, 2023
Merged

Use default page size for jumping to desired offset #38457

merged 2 commits into from
Jun 22, 2023

Conversation

come-nc
Copy link
Contributor

@come-nc come-nc commented May 25, 2023 •
edited
Loading

Summary

Instead of using the final limit as page size, use the default one, which may be higher.
For instance occ ldap:search --limit=2 --offset=100 "" will now do 2 requests instead of 50.

Checklist

@come-nc come-nc added this to the Nextcloud 28 milestone May 25, 2023
@come-nc come-nc self-assigned this May 25, 2023
@come-nc come-nc added 3. to review Waiting for reviews and removed 2. developing Work in progress labels May 25, 2023
@come-nc come-nc requested review from susnux, blizzz, a team, ArtificialOwl and icewind1991 and removed request for a team May 25, 2023 15:43
susnux
susnux approved these changes May 26, 2023
View reviewed changes
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

Code looks good to me

blizzz
blizzz approved these changes Jun 20, 2023
View reviewed changes
Copy link
Member

@blizzz blizzz left a comment

Choose a reason for hiding this comment

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

makes sense 👍

@blizzz
Copy link
Member

blizzz commented Jun 22, 2023

also tested now successfully against Samba4.

@blizzz blizzz merged commit 2e81a1d into master Jun 22, 2023
@blizzz blizzz deleted the fix/improve-ldap-offset-search-perf branch June 22, 2023 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blizzz blizzz blizzz approved these changes

@susnux susnux susnux approved these changes

@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl ArtificialOwl was automatically assigned from nextcloud/server-backend

@icewind1991 icewind1991 Awaiting requested review from icewind1991 icewind1991 was automatically assigned from nextcloud/server-backend

Assignees

@come-nc come-nc

Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Milestone
Nextcloud 28
Development

Successfully merging this pull request may close these issues.

Improve support for offset in ldap searches
3 participants
@come-nc @blizzz @susnux