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.

Sign up for GitHub

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

Jump to bottom

Remove owner_index table from nodes #7878

Merged
merged 1 commit into from
Jan 30, 2023
Merged

Remove owner_index table from nodes #7878

merged 1 commit into from
Jan 30, 2023

Conversation

lxfind
Copy link
Contributor

@lxfind lxfind commented Jan 30, 2023 •
edited
Loading

Indexer covers this now.

@vercel
Copy link

vercel bot commented Jan 30, 2023

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

4 Ignored Deployments
Name Status Preview Updated
explorer ⬜️ Ignored (Inspect) Jan 30, 2023 at 6:32AM (UTC)
explorer-storybook ⬜️ Ignored (Inspect) Jan 30, 2023 at 6:32AM (UTC)
frenemies ⬜️ Ignored (Inspect) Jan 30, 2023 at 6:32AM (UTC)
wallet-adapter ⬜️ Ignored (Inspect) Jan 30, 2023 at 6:32AM (UTC)

@lxfind lxfind changed the title Remove owner_index table Remove owner_index table from nodes Jan 30, 2023
@lxfind lxfind marked this pull request as ready for review January 30, 2023 15:50
@gegaowp
Copy link
Contributor

gegaowp commented Jan 30, 2023

are we going to merge this soon? iiuc today RPC's getObjectsOwnedByAddress is enabled by this table, it's on the plan to let indexer to take over this indeed, but the rough timeline is end of Q1.

@lxfind
Copy link
Contributor Author

lxfind commented Jan 30, 2023

Could you point to me where in the RPC handling we are using this table?

mystenmark
mystenmark approved these changes Jan 30, 2023
View reviewed changes
Copy link
Contributor

@mystenmark mystenmark left a comment

Choose a reason for hiding this comment

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

I am surprised that this is actually not used - but if we're sure its not, very much approved.

@lxfind lxfind requested a review from patrickkuo January 30, 2023 16:53
@patrickkuo
Copy link
Contributor

are we going to merge this soon? iiuc today RPC's getObjectsOwnedByAddress is enabled by this table, it's on the plan to let indexer to take over this indeed, but the rough timeline is end of Q1.

I have moved this to the index store instead of using the one in Authority store quite a while ago, the only place using this is handle_account_info_request in authority client, which is not need anymore.

@lxfind lxfind merged commit 7597b09 into main Jan 30, 2023
@lxfind lxfind deleted the db-remove-owned-index-table branch January 30, 2023 16:58
williampsmith pushed a commit that referenced this pull request Feb 3, 2023
@lxfind @williampsmith
Remove owner_index table from nodes (#7878)
f748d70 
Indexer covers this now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mystenmark mystenmark mystenmark approved these changes

@gegaowp gegaowp Awaiting requested review from gegaowp

@phoenix-o phoenix-o Awaiting requested review from phoenix-o

@sadhansood sadhansood Awaiting requested review from sadhansood

@patrickkuo patrickkuo Awaiting requested review from patrickkuo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lxfind @gegaowp @patrickkuo @mystenmark