Move DB logic to firefly-common impl #1110
Conversation
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
peterbroadhurst
force-pushed
the
dbcommon
branch
from
a116f08 to
24c6480
Compare
Codecov Report
@@ Coverage Diff @@
## main #1110 +/- ##
==========================================
+ Coverage 99.93% 99.97% +0.04%
==========================================
Files 309 302 -7
Lines 20546 19426 -1120
==========================================
- Hits 20532 19422 -1110
+ Misses 10 4 -6
+ Partials 4 0 -4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
nguyer
left a comment
nguyer
left a comment
There was a problem hiding this comment.
Just a couple of questions, but otherwise very straightforward. Looks good.
| mock.ExpectBegin().WillReturnError(fmt.Errorf("pop")) | ||
| err := s.InsertBlob(context.Background(), &core.Blob{}) | ||
| assert.Regexp(t, "FF10114", err) | ||
| assert.Regexp(t, "FF00175", err) |
There was a problem hiding this comment.
There are a lot of error codes that have changed throughout this PR. Is there a migration consideration here?
There was a problem hiding this comment.
With our current architecture for errors, moving them between repos, means changing them - as the prefixes are reserved in each repo.
There was a problem hiding this comment.
I've added the migration_consideration tag so we can include it the release notes
| return mp.Name() | ||
| } | ||
|
|
||
| func (psql *mockProvider) Features() SQLFeatures { |
There was a problem hiding this comment.
Question on this file. Do we need this anymore? internal/database/sqlcommon/provider.go was removed above, so I'm wondering if we need the mock test anymore? Or should this be in firefly-common?
There was a problem hiding this comment.
We do - we have a mix of tests that use SQLite for an E2E "real" test, like TestBatch2EWithDB, and others that fill out coverage by testing edge cases using the mock provider.
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
3 participants
Core half of hyperledger/firefly-common#39