QA (public) » openQA Project (public)

I think one important question here needs to be answered before this can move forward. Are we ok with simply informing the user that the maximum number of results is returned? Or do we need to know if there really are more results? Because the latter is really hard and will require most SQL queries to be modified in significant ways. I tend to use the PostgreSQL COUNT(*) OVER() feature for this, but including it in DBIx::Class queries would be tricky.

Edit: This is not true for all queries. While going over the various limits, i noticed that many are simple enough that we could detect more results simply by requesting one more than the limit (and then removing it manually from the resultset). Pagination is still the proper solution though.

kraih wrote:

I think one important question here needs to be answered before this can move forward. Are we ok with simply informing the user that the maximum number of results is returned? Or do we need to know if there really are more results? Because the latter is really hard and will require most SQL queries to be modified in significant ways. I tend to use the PostgreSQL COUNT(*) OVER() feature for this, but including it in DBIx::Class queries would be tricky.

Well, the first would be a small improvement which you can introduce as a first PR of course. but it's a logical consequence that one would be interested to know if there would be more results or not so we need the second step anyway.

Well, the first would be a small improvement which you can introduce as a first PR of course. but it's a logical consequence that one would be interested to know if there would be more results or not so we need the second step anyway.

In that case it would probably be easier and more user friendly to just do pagination. Still a larger task though, that should probably be an epic with subtasks for each API endpoint that needs pagination.

kraih wrote:

I think one important question here needs to be answered before this can move forward. Are we ok with simply informing the user that the maximum number of results is returned? Or do we need to know if there really are more results? Because the latter is really hard and will require most SQL queries to be modified in significant ways. I tend to use the PostgreSQL COUNT(*) OVER() feature for this, but including it in DBIx::Class queries would be tricky.

AC1 says When data on query or API routes is limited the user is informed that not all results are returned. So no. The goal is to let the user know that data is missing.

cdywan wrote:

AC1 says When data on query or API routes is limited the user is informed that not all results are returned. So no. The goal is to let the user know that data is missing.

What the AC says is not in question. The ticket has been estimated wrong if that is really the goal. So i'm asking if we would be ok with a simplified solution that fits the ticket scope.

Hm, then I suggest not continue with that approach and rather do proper pagination

okurz wrote:

Hm, then I suggest not continue with that approach and rather do proper pagination

That would also be my preference.

Not sure what we were thinking when we estimated this ticket. It is quite unclear which API endpoints even need pagination/limit information. All the linked tickets only have partial data. My starting point would probably be the limit settings, and then a search for the individual limits. That is a lot of endpoints, with very different query implementations.

So lets propose some next steps:

• This ticket is getting rejected, and a new epic created for API pagination.
• Then a linked ticket is created for a select API endpoint to work out the specifics of the pagination API (headers etc.). Which can then be applied to other API endpoints as well. The obvious first choice would be GET /api/v1/assets from #120315.
• Another ticket to make a list of all API endpoints that need pagination and to create more linked tickets.
• Maybe as a followup some additions to the client to take advantage of Link: next=... headers.

Tests and actual API (headers/query params) should be quite similar between the API endpoints, what complicates this is all the different database queries.

kraih wrote:

So lets propose some next steps:

• This ticket is getting rejected, and a new epic created for API pagination.
• Then a linked ticket is created for a select API endpoint to work out the specifics of the pagination API (headers etc.).

Good idea but please in opposite order ;)

We also have the ticket
#119428 about pagination so maybe just add or reference your suggestions there

okurz wrote:

Good idea but please in opposite order ;)

We also have the ticket
#119428 about pagination so maybe just add or reference your suggestions there

Sounds good.

Btw. I think /api/v1/job_settings/jobs with the job_settings_max_recent_jobs limit will be the toughest case here. It might be impossible to add pagination or to show the limit triggered.

That's correct but here I think we can go a slightly different approach. It wouldn't be "pagination" but rather shifting the search window. We limit by job id, right? So we should offer to look deeper into the past on request

okurz wrote:

That's correct but here I think we can go a slightly different approach. It wouldn't be "pagination" but rather shifting the search window. We limit by job id, right? So we should offer to look deeper into the past on request

We can move the window, but we cannot tell the user if there are more results to be found. Which doesn't really help with the problem you ran into earlier where you only got 2 of 257 results. All you could do in that case is try more windows of 20000 job ids, again and again. Of course it's better than nothing.