-
Notifications
You must be signed in to change notification settings - Fork 16
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
pkg/sqlutil/pg: create package; expand env config #450
base: main
Are you sure you want to change the base?
Conversation
pkg/sqlutil/pg/connection.go
Outdated
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.
- revisit this file since core has changed since this was started
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.
Why move some of this postgres setup over to common? Core no longer needs to control all the connection strings that are passed to LOOPs?
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.
What sort of control did you have in mind?
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.
I think I understand what @patrickhuie19 was asking here and can answer it:
For the short term, the evm, cosmos, & solana schemas are being kept in the main chainlink db, but the long term plan is that each relay will have its own separate db. @krehermann worked out a system (at least partially?) where goose migrations can be managed by each chain separately (even different evm chains), the implementation of which I think is the main blocker for each relay having its own db. Once each relay has its own db it will also use its own config to determine the connection string to it, rather than receiving it from the core executable. This PR seems like it gets us a step closer to doing that, but there are other pieces that have to fall into place before we can do that.
pkg/sqlutil/pg/connection_test.go
Outdated
) | ||
|
||
func Test_disallowReplica(t *testing.T) { | ||
db, err := sqlx.Open(string(DriverTxWrappedPostgres), uuid.New().String()) |
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.
Is this test working already? Or is the plan for it to work once the PR that moves txdb into common is merged?
(As I understand it, this shouldn't work unless sql.Register()
is called first, to register the txdb driver and implement it.)
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.
It is moved from core so it is a working test, but it is behind a build tag so it is not enabled in CI currently (or when invoking plain go test
). I don't recall having a particular plan, but we can activate it with your PR
Supports: