etcd/path: change leaderPath/dcLocationPath/memberPath/allocalIDPath to absolute path #8789
Conversation
Signed-off-by: okJiang <[email protected]>
Signed-off-by: okJiang <[email protected]>
Signed-off-by: okJiang <[email protected]>
Signed-off-by: okJiang <[email protected]>
ti-chi-bot
bot
added
release-note-none
okJiang
changed the title
ti-chi-bot
bot
added
the
size/L
Signed-off-by: okJiang <[email protected]>
Signed-off-by: okJiang <[email protected]>
ti-chi-bot
bot
added
the
do-not-merge/work-in-progress
Signed-off-by: okJiang <[email protected]>
ti-chi-bot
bot
added
size/XL
Signed-off-by: okJiang <[email protected]>
|
/retest |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8789 +/- ##
==========================================
- Coverage 75.21% 75.19% -0.02%
==========================================
Files 459 460 +1
Lines 71843 71867 +24
==========================================
+ Hits 54035 54042 +7
- Misses 14351 14369 +18
+ Partials 3457 3456 -1
Flags with carried forward coverage won't be shown. Click here to find out more. |
ti-chi-bot
bot
removed
the
do-not-merge/work-in-progress
|
/retest |
| @@ -488,17 +484,9 @@ func (m *EmbeddedEtcdMember) SetMemberDeployPath(id uint64) error { | |||
| return nil | |||
| } | |||
|
|
|||
| func (m *EmbeddedEtcdMember) getMemberGitHashPath(id uint64) string { | |||
There was a problem hiding this comment.
Do we need to add a test to check if the changes produce the same results as before? For example, we could output the path from the old version as a dataset for the new code test.
There was a problem hiding this comment.
All I can think of is to verify the consistency of the paths through manual testing. There doesn't seem to be any good testing methods to automate🤔
There was a problem hiding this comment.
Maybe we can use fixed testdata from your manual testing.
Signed-off-by: okJiang <[email protected]>
Signed-off-by: okJiang <[email protected]>
Signed-off-by: okJiang <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nolouch The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[LGTM Timeline notifier]Timeline:
|
| @@ -28,6 +28,8 @@ tidb_download_url="${file_server_url}/download/builds/pingcap/tidb/${tidb_sha1}/ | |||
| tikv_download_url="${file_server_url}/download/builds/pingcap/tikv/${tikv_sha1}/centos7/tikv-server.tar.gz" | |||
| tiflash_download_url="${file_server_url}/download/builds/pingcap/tiflash/${branch}/${tiflash_sha1}/centos7/tiflash.tar.gz" | |||
|
|
|||
| ETCD_VERSION=v3.5.9 | |||
There was a problem hiding this comment.
It is used to download etcdctl
| "/pd//raft/r/", | ||
| "/pd//raft/s/", | ||
| "/pd//raft/status/raft_bootstrap_time", | ||
| "/pd//region_label/keyspaces/", |
There was a problem hiding this comment.
Can you explain the difference between // and /?
There was a problem hiding this comment.
To ensure consistency in each test, I removed the numerical parts from the key
| } | ||
|
|
||
| // LeaderPath returns the leader path. | ||
| func LeaderPath(p *MsParam) string { |
There was a problem hiding this comment.
How about making the parameter optional?
There was a problem hiding this comment.
I prefer to keeping this way. Because it is only called in a few places, and it is wrapped by its upper layer yet.
Signed-off-by: okJiang <[email protected]>
|
/retest |
What problem does this PR solve?
Issue Number: Ref #8582
What is changed and how does it work?
Check List
Tests
Release note