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

feat(filecoin-api): paginated queries #1521

Merged
merged 5 commits into from
Jul 23, 2024
Merged

feat(filecoin-api): paginated queries #1521

merged 5 commits into from
Jul 23, 2024

Conversation

alanshaw
Copy link
Member

@alanshaw alanshaw commented Jul 22, 2024

This PR updates filecoin-api to add pagination to the query interfaces. Currently only the first page of results is returned.

This means in the fileocin pipeline cron jobs we never process more than 1 page of items, even though there are sometimes many more items to process and ample execution time remaining in the lambda.

If only the first page of items is processed it gives a false sense of everything operating ok, when actually things may be backing up. It essentially smothers the issue. If the lambda times out because there are too many items to process then this is a good indication that it needs some attention.

Essentially, the interface changes from:

interface Queryable<Q, T> {
  query (q: Q): Promise<Result<T[]>>
}

To this:

interface Pageable {
  cursor?: string
  size?: number
}

interface ListSuccess<T> {
  results: T[]
  cursor?: string
}

interface Queryable<Q, T> {
  query (q: Q, options?: Pageable): Promise<Result<ListSuccess<T>>>
}

Context: we currently have a lot of filecoin/accept receipts we need to issue that are being done in a cron job.

InclusionRecordQueryByGroup
>
export type InclusionStore = Store<InclusionRecordKey, InclusionRecord> &
QueryableStore<InclusionRecordQueryByGroup, InclusionRecord>
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 flattened the store types to give us more flexibility and simplicity. They can be composed as shown and we now have the ability to express in types a read only store for querying data (for example).

Copy link
Member

Choose a reason for hiding this comment

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

+1 here for composable types :)

Copy link
Member

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

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

LGTM

Overall this looks really good. Appreciate the type changes and the paging interface looks really good. Do we have adequate integration testing to insure a safe deploy?

InclusionRecordQueryByGroup
>
export type InclusionStore = Store<InclusionRecordKey, InclusionRecord> &
QueryableStore<InclusionRecordQueryByGroup, InclusionRecord>
Copy link
Member

Choose a reason for hiding this comment

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

+1 here for composable types :)

@alanshaw
Copy link
Member Author

Do we have adequate integration testing to insure a safe deploy?

Yeah, I'm confident this is safe to deploy. We already have tests that cover the code that uses .query(...). I added coverage reporting to double check:

Screenshot 2024-07-23 at 10 30 52 Screenshot 2024-07-23 at 10 29 54 Screenshot 2024-07-23 at 10 28 35

@alanshaw alanshaw merged commit 25ed7d7 into main Jul 23, 2024
15 checks passed
@alanshaw alanshaw deleted the feat/paginated-queries branch July 23, 2024 09:34
alanshaw pushed a commit that referenced this pull request Jul 23, 2024
🤖 I have created a release *beep* *boop*
---


##
[7.2.0](filecoin-api-v7.1.1...filecoin-api-v7.2.0)
(2024-07-23)


### Features

* **filecoin-api:** paginated queries
([#1521](#1521))
([25ed7d7](25ed7d7))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

2 participants