-
Notifications
You must be signed in to change notification settings - Fork 117
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
Add federation sync configuration SQL table and RPC endpoints #537
Conversation
43591a1
to
fd02dbd
Compare
010689b
to
76571f6
Compare
76571f6
to
4f42c0a
Compare
All set/query db transactions are now atomic. I've added an enum to signify which proofs should be synced. I think it's the cleanest solution because each sync process will either exclude proofs, sync issuance, or sync all proofs. We can also expand the enum (which comes close to Oli's key-value idea.) |
eacca37
to
f38d319
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good. I think the level of configurability makes sense for the moment.
I assume the code to actually sync according to the setting will be done in a follow-up PR?
|
||
// QueryFederationSyncConfigs returns the general and universe specific | ||
// federation sync configs. | ||
func (u *UniverseFederationDB) QueryFederationSyncConfigs( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A standalone unit test for DB operations would be nice.
e037319
to
e05294b
Compare
Yep plan is to do this as we work through #544. |
Needs a rebase! |
e05294b
to
2af624b
Compare
7b813c1
to
5557ce6
Compare
5557ce6
to
05da767
Compare
b1c6588
to
3ab1c01
Compare
54f716f
to
0dcd95c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎉
fe93d2c
to
7f6d3fa
Compare
8544884
to
3616bd6
Compare
This commit adds two tables for storing universe federation sync configuration settings: 1. federation_global_sync_config stores global (but proof type specific) federation sync configuration. 2. federation_uni_sync_config stores universe specific sync configuration.
Add RPC endpoints for setting and querying for federation sync config: * SetFederationSyncConfig * QueryFederationSyncConfig
3616bd6
to
95b03be
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 3 of 16 files at r5, all commit messages.
Reviewable status: 9 of 22 files reviewed, 37 unresolved discussions (waiting on @ffranr, @guggero, and @jharveyb)
rpcserver.go
line 2598 at r4 (raw file):
return &universe.FedUniSyncConfig{ UniverseID: uniID, ProofTypes: proofTypes,
I think we still want this here to distinguish between issuance/transfer trees for insert/export?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🧲
|
||
-- This table contains universe (asset/asset group) specific federation sync | ||
-- configuration. | ||
CREATE TABLE IF NOT EXISTS federation_uni_sync_config ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC, in order for something to later on have a foreign key that refs this table, it does in fact still need a primary key.
I think we just want a composite primary key here. Though I saw you had to re-work things slightly tho, non-blocking comment.
This PR adds functionality to configure the universe federation sync routine. It facilitates both a general configuration and a universe/asset specific configuration.
This PR is part of the work necessary to complete #530 .
This change is