feat(page): add page param to support pagination without knowing dids #254
Conversation
|
The style in this PR agrees with This formatting comment was generated automatically by a script in uc-cdis/wool. |
|
This pull request introduces 1 alert and fixes 2 when merging fec1b25 into 2d43b97 - view on LGTM.com new alerts:
fixed alerts:
|
indexd/index/drivers/alchemy.py
Outdated
| query = query.order_by(IndexRecord.updated_date) | ||
| else: | ||
| query = query.order_by(IndexRecord.did) |
There was a problem hiding this comment.
should we just always order by date?
There was a problem hiding this comment.
I thought about that... probably? I was afraid to change the existing behavior tbh, but I guess it shouldn't matter, you'll always get the next did regardless of the ordering
indexd/index/drivers/alchemy.py
Outdated
| # (e.g. parallelly processing from beginning -> middle and ending -> middle | ||
| # and as a final step, checking the "ending"+1 to see if there are | ||
| # new records). | ||
| query = query.order_by(IndexRecord.updated_date) |
There was a problem hiding this comment.
why not order by decreasing date? if we use this to validate after indexing for example, we could just get the first pages
There was a problem hiding this comment.
The way this is set the dates later in the past will be first such that any additional records end up at the end. This means that existing pages won't change content, whereas if we were to do the opposite, a new record would bump everything else forward, changing the content on all existing pages. So I didn't want to break things mid-processing. If you're curious, this is the logic I was cross-testing with gen3 sdk: https://github.com/uc-cdis/gen3sdk-python/blob/907d418ebcf46ea345472229982c604c54cce179/gen3/tools/indexing.py#L82
There was a problem hiding this comment.
makes sense!! thanks
New Features
pageparam to list records endpoint to support pagination without using a didBreaking Changes
Bug Fixes
Improvements
Dependency updates
Deployment changes