Refactor etcd config as discovery section with convenience types #2843
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2843 +/- ##
========================================
- Coverage 72.1% 72.1% -0.1%
========================================
Files 1101 1101
Lines 101630 99857 -1773
========================================
- Hits 73351 72009 -1342
+ Misses 23319 22909 -410
+ Partials 4960 4939 -21
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #2843 +/- ##
======================================
Coverage 72.1% 72.1%
======================================
Files 1099 1099
Lines 99827 99827
======================================
Hits 72006 72006
Misses 22888 22888
Partials 4933 4933
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
robskillington
changed the base branch from
master
to
refactor-config-use-defaults-wherever-possible
…/etcd-config-as-discovery-section
| case M3DBSingleNodeType: | ||
| return "m3db_single_node" | ||
| case M3DBClusterType: | ||
| return "m3db_cluster" |
There was a problem hiding this comment.
Do we need to disambiguate between these two cases at this level? Can we do the right thing based on whether the provided cluster config has 1 or >1 entries?
There was a problem hiding this comment.
Unfortunately not, for m3db_single_node case the etcd configuration is not specified (to make this reduced config possible). When m3db_single_node is set it knows without specifying the local host address as the etcd to connect to (i.e. you haven't connected to etcd yet to infer that there's a single node present, you need to inform DB that you are expecting a single node and then it can follow the behavior of connecting to etcd on the embedded DB node instead of making you type out in config the etcd address).
There was a problem hiding this comment.
Ah, I see what you mean now - makes sense.
…/m3 into ra/etcd-config-as-discovery-section
.golangci.yml
Outdated
| # New line required before return would require a large fraction of the | ||
| # code base to need updating, it's not worth the perceived benefit. | ||
| - nlreturn |
There was a problem hiding this comment.
Let's cut this separately, please
* master: (28 commits) [dbnode] Add claims for index segments volume index (#2846) [dbnode] Remove namespaces from example config and integration tests (#2866) [dbnode] Resurrect flaky test skip (#2868) [aggregator] Fix checkCampaignStateLoop (#2867) [dbnode] implement deletion method in namespace kvadmin service (#2861) Replace closer with resource package (#2864) Add coding style guide (#2831) Add GOVERNANCE.md to describe governance (#2830) Add COMPATIBILITY.md to describe version compatibility (#2829) Refactor etcd config as discovery section with convenience types (#2843) Refactor x/lockfile into dbnode/server (#2862) [lint] Disable nlreturn linter (#2865) [m3cluster] Expose placement algorithm in placement service (#2858) [etcd] Set reasonable cluster connection/sync settings by default (#2860) [dbnode] Use bits.LeadingZeros64 to improve encoder performance (#2857) Cleanup m3nsch leftovers (#2856) Update ci-scripts to correct coverage tracking (#2854) [aggregator] Process campaign state without waiting for first campaign check interval (#2855) Bump go to 1.14 (#2853) [query] Remove single series error from M3 ...
What this PR does / why we need it:
This change makes the etcd config easier to define in the common use-cases with default values. These include
And then we have the traditional definition still supported via
Special notes for your reviewer:
Does this PR introduce a user-facing and/or backwards incompatible change?:
Does this PR require updating code package or user-facing documentation?: