Remove default in orc8r database storage#7484
Conversation
|
Thanks for opening a PR! 💯 Please note that all commits must be signed off. This is enforced by the Howto
More infoPlease take a moment to read through the Magma project's
If this is your first Magma PR, also consider reading
|
hcgatewood
left a comment
hcgatewood
left a comment
There was a problem hiding this comment.
Thanks for the change! Couple small details then ready to go
orc8r/cloud/go/test_utils/test_db.go
Outdated
| func GetSharedMemoryDB() (*sql.DB, error) { | ||
| var err error | ||
| once.Do(func() { instance, err = sqorc.Open(storage2.SQLDriver, ":memory:") }) | ||
| once.Do(func() { instance, err = sqorc.Open("sqlite3", ":memory:") }) |
There was a problem hiding this comment.
I think there's a const for this somewhere in the codebase. If not, let's add one -- I think there's one for postgres you could look for to emulate
There was a problem hiding this comment.
couldn't find the postgres const so I moved it to the definitions const file
Codecov Report
@@ Coverage Diff @@
## master #7484 +/- ##
=======================================
Coverage 64.43% 64.43%
=======================================
Files 679 679
Lines 46973 46975 +2
Branches 1320 1320
=======================================
+ Hits 30265 30268 +3
Misses 13259 13259
+ Partials 3449 3448 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Signed-off-by: Eli Sutton <[email protected]>
Move SQLDriver and DatabaseSource from global vars to getter funcs. Signed-off-by: Eli Sutton <[email protected]>
pull-request-size
bot
added
size/M
hcgatewood
left a comment
hcgatewood
left a comment
There was a problem hiding this comment.
Looks good, couple notes
Signed-off-by: Eli Sutton <[email protected]>
hcgatewood
left a comment
hcgatewood
left a comment
There was a problem hiding this comment.
One last aesthetic fix then am ready to merge
Signed-off-by: Eli Sutton <[email protected]>
hcgatewood
left a comment
hcgatewood
left a comment
There was a problem hiding this comment.
Lgtm, thanks for the fix!
You'll need to re-push (e.g. force-push a noop commit or amend the head commit) to re-kick off the failed flaky cirlceci job
| } | ||
|
|
||
| // MustGetEnv returns the string value of the environment variable, | ||
| // panics it doesn't exist |
There was a problem hiding this comment.
Nit
| // panics it doesn't exist | |
| // panics it doesn't exist. |
Signed-off-by: Ramon Melero <[email protected]>
4 participants
Remove default in orc8r database storage
Summary
Add a new MustGetEnv function in orc8r/lib/go/definitions/env.go
Set SQL driver and DB source in orc8r/cloud/go/storage/storage.go with MustGetEnv instead of the "with default" getter
Fix tests
Test Plan
./build.py -t
Additional Information
#4086