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 using limit=0 and details=on to get only the count of elements (prelanding) #4104

Merged
merged 34 commits into from
Apr 28, 2022

Conversation

fgalan
Copy link
Member

@fgalan fgalan commented Apr 27, 2022

Continues the work in PR #3994 . Issue #1492

Note that just after creating the PR (at commit c20bd31) the diff is +707 -62 deletions, exactly as in the original PR.

CC: @Anjali-NEC . Please pay attention to any further comment or modification I'll do in this PR.

Anjali-NEC and others added 26 commits November 9, 2021 13:37
Fix using limit=0 and details=on to get only the count of elements #1492
"code": "400",
"details": "Bad pagination limit: /000000/ [a value of ZERO is unacceptable]",
"details": "JSON Parse Error",
Copy link
Member Author

Choose a reason for hiding this comment

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

I think I finally found the reason for this "JSON Parse Error" :)

payload='NO PAYLOAD NECESSARY'

Let's try to modify this tests with a proper queryContext JSON request payload...

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in ae956ec

@fgalan
Copy link
Member Author

fgalan commented Apr 28, 2022

I have simplified the code so for instance instead of

if (limit != 0)
{
   while ((limit != 0) && (cursor.next(&r, &errType, &nextErr)))
   ...

we have a more compact equivalent option:

while ((limit != 0) && (cursor.next(&r, &errType, &nextErr)))

to avoid unnecessary re-indentation of code blocks.

@fgalan
Copy link
Member Author

fgalan commented Apr 28, 2022

Please @Anjali-NEC have a look to the final state of this PR and if you find it ok provide LGTM so we can merge it.

@fgalan fgalan requested a review from mapedraza April 28, 2022 14:13
Copy link
Collaborator

@mapedraza mapedraza left a comment

Choose a reason for hiding this comment

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

LGTM

@Anjali-NEC
Copy link
Contributor

Please @Anjali-NEC have a look to the final state of this PR and if you find it ok provide LGTM so we can merge it.

LGTM

@mapedraza mapedraza merged commit cb1e7d3 into master Apr 28, 2022
@mapedraza mapedraza deleted the issue1492-prelanding branch April 28, 2022 15:46
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.

3 participants