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

graphql: add flag to skip db compatibility #19671

Merged
merged 3 commits into from
Oct 3, 2024
Merged

graphql: add flag to skip db compatibility #19671

merged 3 commits into from
Oct 3, 2024

Conversation

amnn
Copy link
Member

@amnn amnn commented Oct 2, 2024

Description

Add a flag to optionally skip database migration compatibility checks. This is helpful when trying to connect a local build up to a production database to test a specific change, even if you are aware that other queries may not be compatible.

To accommodate this change, the compatibility check was also moved into ServerBuilder where the config is readily available. This also slightly simplifies the Server itself, which no longer needs to hold onto its own instance of the Db.

Test plan

Connected a local build of GraphQL to the production DB, which it is not compatible with.

Stack


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.

  • Protocol:
  • Nodes (Validators and Full nodes):
  • Indexer:
  • JSON-RPC:
  • GraphQL: Add --skip-migration-consistency-check to allow bypassing the database compatibility checks.
  • CLI:
  • Rust SDK:
  • REST API:

@amnn amnn requested review from lxfind and bmwill October 2, 2024 22:18
@amnn amnn self-assigned this Oct 2, 2024
Copy link

vercel bot commented Oct 2, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sui-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 3, 2024 10:42pm
3 Skipped Deployments
Name Status Preview Comments Updated (UTC)
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Oct 3, 2024 10:42pm
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Oct 3, 2024 10:42pm
sui-typescript-docs ⬜️ Ignored (Inspect) Visit Preview Oct 3, 2024 10:42pm

## Description

Avoid duplicating fields for configs that are accepted as flags from the
command-line and can also be read from TOML file. Use the same struct
for both purposes but deocrate it with both `clap` and `serde` (or
`GraphQLConfig`) annotations so that it can serve both purposes.

## Test plan

This change should preserve the config/command-line interface -- checked
by running the GraphQL service with the same invocation, referencing all
flags, before and after the change.
## Description

Add a flag to optionally skip database migration compatibility checks.
This is helpful when trying to connect a local build up to a production
database to test a specific change, even if you are aware that other
queries may not be compatible.

To accommodate this change, the compatibility check was also moved into
`ServerBuilder` where the config is readily available. This also
slightly simplifies the `Server` itself, which no longer needs to hold
onto its own instance of the `Db`.

## Test plan

Connected a local build of GraphQL to the production DB, which it is not
compatible with.
@amnn amnn mentioned this pull request Oct 3, 2024
8 tasks
Base automatically changed from amnn/gql-config-clean to main October 3, 2024 23:12
@amnn amnn merged commit 8213cfe into main Oct 3, 2024
45 of 47 checks passed
@amnn amnn deleted the amnn/gql-skip-compat branch October 3, 2024 23:12
amnn added a commit that referenced this pull request Oct 3, 2024
## Description

Make it so that the module that is associated as the "sending module"
for an event is relocated by linkage. This fixes an issue (captured in a
regression test) where if the function that is called by the PTB is from
some upgraded version of the package, it cannot be found because the
Event's package ID still points to the original version of the package.

## Test plan

Introduced a regression test -- before the change, the "sending module"
in the response returned `null`.

```
sui$ cargo nextest run -p sui-graphql-e2e-tests -- sending_module
```

## Stack

- #19670 
- #19671 

---

## 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.

- [x] Protocol: Protocol version bump to 62, changing the semantics
around the package/module in the PTB that the event originated from.
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
- [ ] REST API:
amnn added a commit that referenced this pull request Oct 10, 2024
## Description

Add the ability to fetch the transaction block that emitted the event.

## Test plan

New E2E tests:

```
sui$ cargo nextest run -p sui-graphql-e2e-tests
sui$ cargo nextest run -p sui-graphql-rpc -- test_schema_sdl_export
sui$ cargo nextest run -p sui-graphql-rpc --features staging -- test_schema_sdl_export
```

## Stack

- #19670 
- #19671 
- #19672 

---

## 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.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [x] GraphQL: Add `Event.transactionBlock` for fetching the transaction
that emitted the event, as long as the event is indexed (not just
executed).
- [ ] CLI: 
- [ ] Rust SDK:
- [ ] REST API:
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