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

[mvr] use mvr-mode on sui-mvr-graphql-rpc for in-crate tests #20318

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

wlmyng
Copy link
Contributor

@wlmyng wlmyng commented Nov 19, 2024

Description

Needed to adjust the ConnectionAsObjectStore, because objects will not be written to in mvr-mode. Querying objects and querying objects_history + objects_snapshot should both yield the live object set.

Test plan

How did you test the new or updated feature?


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:
  • CLI:
  • Rust SDK:
  • REST API:

Copy link

vercel bot commented Nov 19, 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 Nov 19, 2024 10:54pm
3 Skipped Deployments
Name Status Preview Comments Updated (UTC)
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Nov 19, 2024 10:54pm
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Nov 19, 2024 10:54pm
sui-typescript-docs ⬜️ Ignored (Inspect) Visit Preview Nov 19, 2024 10:54pm

@wlmyng wlmyng temporarily deployed to sui-typescript-aws-kms-test-env November 19, 2024 01:27 — with GitHub Actions Inactive
@wlmyng wlmyng temporarily deployed to sui-typescript-aws-kms-test-env November 19, 2024 04:46 — with GitHub Actions Inactive
@wlmyng wlmyng marked this pull request as ready for review November 19, 2024 04:48
@wlmyng wlmyng temporarily deployed to sui-typescript-aws-kms-test-env November 19, 2024 04:48 — with GitHub Actions Inactive
@wlmyng wlmyng requested a review from gegaowp November 19, 2024 04:48
@wlmyng wlmyng changed the title use mvr-mode on sui-mvr-graphql-rpc [mvr] use mvr-mode on sui-mvr-graphql-rpc Nov 19, 2024
@wlmyng wlmyng changed the title [mvr] use mvr-mode on sui-mvr-graphql-rpc [mvr] use mvr-mode on sui-mvr-graphql-rpc for in-crate tests Nov 19, 2024
Copy link
Contributor

@gegaowp gegaowp left a comment

Choose a reason for hiding this comment

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

looks good!
and to double check, this will not affect JSON RPC's object related endpoints, right?

@wlmyng
Copy link
Contributor Author

wlmyng commented Nov 19, 2024

looks good! and to double check, this will not affect JSON RPC's object related endpoints, right?

It might affect IndexerReader's get_latest_sui_system_state and get_validator_from_table since it relies on ConnectionAsObjectStore, but I can revert the change on IndexerReader's get_object_from_db to minimize impact. But even then, reading from objects table should be the same as objects_history + objects_snapshot

@gegaowp
Copy link
Contributor

gegaowp commented Nov 19, 2024

yeah as this change is for mvr-mode only, would be good to minimize changes to other reading paths.

Base automatically changed from sui-mvr-graphql-rpc to main November 19, 2024 22:38
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