Validate datastore engine #810
Conversation
|
Hi @jon-whit, based on our last discussion, raised this PR to verify Datastore engine. I have purposefully setup validation after unmarshalling of configs, as I can't write custom unmarshal for viper. Please have a look. Thanks |
cmd/run/run_test.go
Outdated
|
|
||
| t.Run("correct datastore engine specified", func(t *testing.T) { | ||
| cfg := DefaultConfig() | ||
| cfg.Datastore.URI = "postgresql://localhost:5432" |
There was a problem hiding this comment.
I didn't think postgresql was a valid prefix? The enumerations in cmd/shared.go below define postgres.
This test should return an error, and we should change this test to assert postgres://localhost:5432 does not.
There was a problem hiding this comment.
I think, I accidently set URI in this test. I just wanted to test for correct DatastoreEngine. After changes, I am getting expected result.
Although one thing which I noticed is, on setting wrong postgres URI, RunServer stalls and waits for connection. I think we should fail early. Let me know your thoughts
@jon-whit Addressed comments, plz check. |
Himanshu372
force-pushed
the
valid-datastore-engine
branch
from
ba13dc3
to
fa88048
Compare
|
It appears this PR has been stale for at least 14 days 🗓️. If no action is taken the maintainer team may consider closing the PR. Please reach out if you need assistance or help to finish the work 👍🏼. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #810 +/- ##
==========================================
+ Coverage 86.05% 86.08% +0.03%
==========================================
Files 85 86 +1
Lines 8048 8064 +16
==========================================
+ Hits 6925 6941 +16
Misses 792 792
Partials 331 331 ☔ View full report in Codecov by Sentry. |
|
Hi @Himanshu372, i apologize for taking so long to review this. I took the liberty of updating this PR with respect to main. I also added some more validations to the datastore configs. |
Description
Added
DatastoreEnginetype which takes up const permissible engine type valuesDatastoreEngineReferences
Addresses comment - #712 (comment)
Closes #681
Review Checklist
main