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

Committees pagination error #2690

Closed
LindsayYoung opened this issue Oct 6, 2017 · 9 comments
Closed

Committees pagination error #2690

LindsayYoung opened this issue Oct 6, 2017 · 9 comments
Assignees
Labels

Comments

@LindsayYoung
Copy link
Contributor

LindsayYoung commented Oct 6, 2017

It looks like the committees endpoint always returns the total count and not the count for the query.

https://api.open.fec.gov/v1/committees/

  • [ ]
@LindsayYoung
Copy link
Contributor Author

What is supposed to happen is that the small requests use an exact count for the count returned and large requests use the count estimate because when we created that endpoint, a schedule A count could take take a minute and we couldn't add that to every call.

This dichotomy broke down when we added efiling endpoints because they use the ApiResource directly instead of the ItemizedResource. It is possible that we could do queries to efiling without a big performance hit, but will have to do some testing first.

It is also possible that the index improvements we have made would make an accurate count in a timely manner, but that would need testing as well.

Ideally, we would be able to set the threshold per endpoint, it seems like we should be able to pass that in as a kwarg, but did not have success with that locally.

@AmyKort
Copy link

AmyKort commented Nov 22, 2017

Thanks, @LindsayYoung . I'll give this to @jwchumley for now.

@AmyKort AmyKort modified the milestones: Sprint 4.4, RBS 1 (Reliability, stability and bugs) Nov 27, 2017
@pkfec
Copy link
Contributor

pkfec commented Nov 29, 2017

@jwchumley && @LindsayYoung
i did some testing on my local, pointing to DEV DB to understand what the issue could be. "count" in the " https://api.open.fec.gov/v1/committees/" is the total number of records returned from the DB.
screen shot 2017-11-28 at 11 35 06 pm

However, when i added a committee_id=C00462390 to the existing query in the browser, only one record was found in DB and count is now set to 1.
screen shot 2017-11-28 at 11 46 38 pm

This is the behavior of API across all the endpoints. This seems to be right.

@girlprogrammer
Copy link

This behavior is not correct. Prior to when we reported this bug, the behavior was for count to include the number of records in the search results. Try adding search parameter ?q=stabenow And the count attribute in the result is still 60388 even though there are not that many Stabenow committees! This makes pagination work incorrectly when searching with the q query parameter, which is primarily how we use this API. Thanks for continuing this investigation.

@LindsayYoung
Copy link
Contributor Author

Thanks for ringing in, @girlprogrammer!

@pkfec and I just looked at this and the problem was that we were returning the count estimate if the estimate was more than 5,000. That makes sense for other endpoints where the exact count can take minutes to execute, but not on small tables like this. We are going to increase the threshold for doing an exact count, so you should return the exact counts for the smaller tables.

We can double check this is working as expected in our development server soon and ring in here.

@girlprogrammer
Copy link

Thank you, @LindsayYoung and @pkfec!

@pkfec
Copy link
Contributor

pkfec commented Nov 30, 2017

Increased the threshold limit from 5000 to 500000. Tested this on my local. The count appears ok now.
Below is the screen shot that shows that exact count for committees:

screen shot 2017-11-30 at 1 52 34 pm

added seach parameter q=stabenow, and the count seems ok.
screen shot 2017-11-30 at 1 52 47 pm

@AmyKort AmyKort closed this as completed Dec 21, 2017
@girlprogrammer
Copy link

Initial reporter here: I have confirmed this is working correctly in production now.

@LindsayYoung
Copy link
Contributor Author

Thank you @girlprogrammer!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants