Refactor code to make place for downgrade logic #13391
Conversation
serathius
force-pushed
the
downgrade-refactor
branch
2 times, most recently
from
a69e964 to
5e08bdf
Compare
| @@ -18,12 +18,13 @@ import ( | |||
| "context" | |||
|
|
|||
| "github.com/coreos/go-semver/semver" | |||
| pb "go.etcd.io/etcd/api/v3/etcdserverpb" | |||
| "go.etcd.io/etcd/api/v3/membershippb" | |||
| "go.etcd.io/etcd/server/v3/storage/backend" | |||
| "go.etcd.io/etcd/server/v3/storage/schema" | |||
| "go.uber.org/zap" | |||
|
|
|||
There was a problem hiding this comment.
Small nit, lets group these by etcd internal and external packages. It's what some editors will auto change so we will see changes for this in the future.
There was a problem hiding this comment.
Yea, hate that. I don't know good solution in Go for that. Python has isort https://github.com/PyCQA/isort that allows defining custom import blocks.
|
|
||
| downgradeInfo := m.s.GetDowngradeInfo() | ||
| if downgradeInfo != nil && downgradeInfo.Enabled { | ||
| // Todo: return the downgrade status along with the error msg |
There was a problem hiding this comment.
This method only returns error, should we already make DowngradeValidate return something else in the signature so we don't change it later?
There was a problem hiding this comment.
This is old TODO taken from API call. Problem is that this error is passed to grpc server to decide what error code to return to cliuent. Problem is that currently it uses error -> rpc code map, which requires errors to be static (need to be a global variables), meaning that they cannot include information like downgrade status.
Fixing this issue would require to rewrite error handling logic for all requests, which is out of scope of this PR.
serathius
force-pushed
the
downgrade-refactor
branch
from
5e08bdf to
6c8e721
Compare
serathius
force-pushed
the
downgrade-refactor
branch
from
6c8e721 to
f92b4f9
Compare
This PR collects set of refactors that are needed before implementing downgrade logic for #13168.
Refactors are split into multiple commits, reviewing one at the time should help with review process. Please let me know if it would make more sense to extract some commits into separate PR.
cc @ptabor, @lilic