chore(graphql): deduplicate configs and clap args #19670
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
amnn
requested review from
wlmyng,
stefan-mysten,
emmazzz and
suiwombat
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
8 tasks
8 tasks
amnn
force-pushed
the
amnn/gql-config-clean
branch
from
86f8b43 to
47f6c02
Compare
## 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.
amnn
force-pushed
the
amnn/gql-config-clean
branch
from
47f6c02 to
0713298
Compare
amnn
added a commit
that referenced
this pull request
Oct 3, 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 - #19670 --- ## 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 `--skip-migration-consistency-check` to allow bypassing the database compatibility checks. - [ ] CLI: - [ ] Rust SDK: - [ ] REST API:
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
clapandserde(orGraphQLConfig) 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.
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.