feat: pagination support for GetTrace endpoint #7438
Conversation
| if request.limit == 0: | ||
| # theres no limit, so we don't need to return a page token | ||
| return None |
There was a problem hiding this comment.
we have a built in default limit so this condition is not correct
| total_items = 0 | ||
| else: | ||
| total_items = sum(len(group.items) for group in item_groups) | ||
| return PageToken(offset=request.page_token.offset + total_items) |
There was a problem hiding this comment.
I don't think this is the correct strategy to use. I think what you want is to use a combination of the largest timestamp and the last item id. You would also need to add an item_id to the order_by this way you don't have to rescan the entire trace you've already scanned
There was a problem hiding this comment.
I decided to implement it using limit/offset rather than filter_offset for the first pass since its simpler and I thought performance isn't too much of a concern at this point. Do you disagree?
There was a problem hiding this comment.
although I imagine if it's a really big trace, we may not be able to fit it into memory to sort it, so I would want to follow up right after this
There was a problem hiding this comment.
I think performance is in fact a concern because we'd be paginating big traces that can't be loaded right now.
I think we push to implement @volokluev 's suggestion right now.
|
should do w page token |
3 participants
this PR implements support for pagination using limit and offset in the GetTrace endpoint. this will be useful for displaying traces with a lot of trace items.