[graphql] add query epochs functionality #19357
Conversation
siomari
requested review from
amnn,
emmazzz,
stefan-mysten,
suiwombat and
wlmyng
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
| } | ||
| } | ||
|
|
||
| //# run-graphql --cursors {"c":5,"e":2} |
There was a problem hiding this comment.
what about a checkpoint viewed at before 5, and trying to view an epoch beyond that checkpoint, and looking at the full state of epochs per that checkpoint? ie c=3, try to view e=4, c=3, query full epochs state starting from epoch 0
|
Actually.. I think we'll also need to create a new index on column At the same time, it feels a bit extraneous to add an entire index just to support the initial query and filter condition. Can we get by with |
There's only ~500 of these so I think an index would be fine (cheap, and the easiest solution). Note that we already use a similar query to do a consistent fetch on epochs -- I think it's also fine to do that in a follow-up PR, sequential scan over 500 records is peanuts. |
| C: object(0,0) | ||
|
|
||
| task 1, line 7: | ||
| //# create-checkpoint |
There was a problem hiding this comment.
nit: if the aim is just to create a checkpoint for the epoch to contain, that isn't necessary, advance-epoch also generates a checkpoint (as you can see, the checkpoints created by create-checkpoint seem to go up in 2's because of this).
| // TODO: Short term hack to get around indexer epoch issue | ||
| //# create-checkpoint | ||
|
|
||
| //# advance-epoch | ||
|
|
||
| // TODO: Short term hack to get around indexer epoch issue | ||
| //# create-checkpoint | ||
|
|
||
| //# advance-epoch | ||
|
|
||
|
|
||
| // TODO: Short term hack to get around indexer epoch issue | ||
| //# create-checkpoint | ||
|
|
||
| //# advance-epoch | ||
|
|
||
| //# create-checkpoint | ||
|
|
||
| //# advance-epoch | ||
|
|
||
| //# create-checkpoint | ||
|
|
||
| //# advance-epoch | ||
|
|
||
| //# create-checkpoint | ||
|
|
||
| //# advance-epoch |
There was a problem hiding this comment.
Per the earier comment
| // TODO: Short term hack to get around indexer epoch issue | |
| //# create-checkpoint | |
| //# advance-epoch | |
| // TODO: Short term hack to get around indexer epoch issue | |
| //# create-checkpoint | |
| //# advance-epoch | |
| // TODO: Short term hack to get around indexer epoch issue | |
| //# create-checkpoint | |
| //# advance-epoch | |
| //# create-checkpoint | |
| //# advance-epoch | |
| //# create-checkpoint | |
| //# advance-epoch | |
| //# create-checkpoint | |
| //# advance-epoch | |
| //# advance-epoch | |
| //# advance-epoch | |
| //# advance-epoch | |
| //# advance-epoch | |
| //# advance-epoch | |
| //# advance-epoch |
There was a problem hiding this comment.
These changes look good @siomari ! There are some CI warnings to address:
- Issue from clippy
- Updating the GraphQL schema (run the test that failed in CI, and it will explain how to update the snapshot).
- You will also need to rebase your PR, because we recently moved the E2E tests into a sub-directory called
stable.
Once that's done, you're good to go!
siomari
force-pushed
the
ms/query-epochs-graphql
branch
from
df90322 to
005f37a
Compare
3 participants
Description
Implementation of paginated query for epochs in GraphQL
Test plan
How did you test the new or updated feature?
Added a pagination.move test under the sui-graphql-e2e-tests crate.
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.