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 suggertaccout 500 limit #92

Closed
wants to merge 1 commit into from
Closed

Conversation

Seizens
Copy link
Contributor

@Seizens Seizens commented Aug 12, 2021

fix suggertaccount 500 limit
AccountInfo struct add MoreAccount parameter to see if the request needs to continue
QueryBody Add Start parameter Query start position

image

 1. 修改SuggestAccount查询500的限制
 2. 添加查询参数start突破查询500的限制

Log
@andygrunwald
Copy link
Owner

Hey @Seizens,
thanks a lot for the PR. At the first glance, it seems reasonable.
However, I want to check if this is somehow related to #48 and if the S parameter is a general issue or can be solved in a general way, rather than for a single endpoint.

I have limited time right now to dig into it. Please bear with me when this takes a bit of time.
Thanks again for your efforts.

@andygrunwald
Copy link
Owner

Hey @Seizens,

thanks for raising the PR.
I merged part of your code, see 101051e.

I introduced the s directly in QueryOptions, because the parameter is also supported in other cases, see list-changes for example: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#list-changes

I hope this will work for you.

@andygrunwald
Copy link
Owner

Maybe I made an implementation mistake with 101051e because of #48

I will fix this in the next days and add proper testing.

@andygrunwald andygrunwald reopened this Sep 15, 2021
@Seizens
Copy link
Contributor Author

Seizens commented Sep 17, 2021

hello @andygrunwald
thanks you !!!
I am honored that my code is useful to you .
I find group.go groupInfo will also have number limits,
you can see https://gerrit-review.googlesource.com/Documentation/rest-api-groups.html#query-groups here , If the number of groups matching the query exceeds either the internal limit or a supplied limit query parameter, the last group object has a _more_groups: true JSON field set.

If I have time, I will also make a new PR, and i think the numbers limits will need other place .

the last , i am sorry, my english is just soso.

@andygrunwald
Copy link
Owner

Hey @Seizens,
thanks again for your involvement.

For SuggestAccount, I have implemented your change in 67c9345

For GroupInfo, I have implemented the _more_groups field in ff14d06

If you encounter other missing features, feel free to open a new issue or a Pull Request. Thanks again.

andygrunwald added a commit that referenced this pull request Sep 19, 2021
* master:
  Groups: Add _more_groups attribute to GroupInfo
  Fix #92: Add pagination support for SuggestAccount
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