[ARCH-330] Add keystore postgres storage#1651
Conversation
pavel-raykov
had a problem deploying
to
integration
GitHub Actions
Error
pavel-raykov
had a problem deploying
to
integration
GitHub Actions
Failure
pavel-raykov
temporarily deployed
to
integration
GitHub Actions
Inactive
✅ API Diff Results - No breaking changes |
pavel-raykov
had a problem deploying
to
integration
GitHub Actions
Failure
pavel-raykov
temporarily deployed
to
integration
GitHub Actions
Inactive
pavel-raykov
had a problem deploying
to
integration
GitHub Actions
Failure
product-security-plaid-production
Bot
requested a review
from cedric-cordenier
pavel-raykov
force-pushed
the
add-pg-storage
branch
from
489e5fa to
0f07604
Compare
pavel-raykov
temporarily deployed
to
integration
GitHub Actions
Inactive
pavel-raykov
had a problem deploying
to
integration
GitHub Actions
Failure
pavel-raykov
had a problem deploying
to
integration
GitHub Actions
Failure
pavel-raykov
had a problem deploying
to
integration
GitHub Actions
Failure
pavel-raykov
had a problem deploying
to
integration
GitHub Actions
Failure
pavel-raykov
temporarily deployed
to
integration
GitHub Actions
Inactive
| "github.com/smartcontractkit/chainlink-common/pkg/sqlutil/sqltest" | ||
| ) | ||
|
|
||
| func TestPgStorage(t *testing.T) { |
There was a problem hiding this comment.
should test the update as well and make sure data changes/timestamp updated, name remains the same etc.
There was a problem hiding this comment.
I have added a part testing the update.
About the updated_at timestamp: currently no business logic uses it and most likely it will stay the same (it should only be used as additional debug information). If we start exposing it, then we should also test it; but for now, to keep it lightweight I think it is fine to not touch created_at and updated_at fields at all.
|
|
||
| var _ keystore.Storage = &Storage{} | ||
|
|
||
| // Storage implements Storage using a Postgres database |
There was a problem hiding this comment.
Is the migration file in core then I guess? Curious if we could keep the migration file here to fully decouple, I could imagine using a PG database for storage outside of the core context (e.g. JD)
There was a problem hiding this comment.
The migration has been added here https://github.com/smartcontractkit/chainlink/blob/b19ae3b2ffaed741930917ce4f20a076b0dcfad3/core/store/migrate/migrations/0280_create_keystore_table.sql#L1 . I guess it will be possible to decouple it, but then we will have to recreate all the CL_DATABASE_URL logic just for this. As the main goal is to keep the implementation lightweight I think it is fine to reuse the main database for now.
pavel-raykov
temporarily deployed
to
integration
GitHub Actions
Inactive
pavel-raykov
had a problem deploying
to
integration
GitHub Actions
Failure
pavel-raykov
had a problem deploying
to
integration
GitHub Actions
Failure
pavel-raykov
had a problem deploying
to
integration
GitHub Actions
Failure
pavel-raykov
had a problem deploying
to
integration
GitHub Actions
Failure
pavel-raykov
had a problem deploying
to
integration
GitHub Actions
Failure
3 participants
The failing race test TestStandardCapabilityCallsAreAsync is unrelated.