-
Notifications
You must be signed in to change notification settings - Fork 527
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[feat] tools-v2: add update leader-schedule[-all] #2551
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good job
tools-v2/pkg/cli/command/curvebs/update/leader_schedule_all/leader_schedule_all.go
Outdated
Show resolved
Hide resolved
tools-v2/pkg/cli/command/curvebs/update/leader_schedule/leader_schedule.go
Outdated
Show resolved
Hide resolved
tools-v2/pkg/cli/command/curvebs/update/leader_schedule_all/leader_schedule_all.go
Outdated
Show resolved
Hide resolved
b32dcd8
to
a97d83b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good job
cicheck |
@Cyber-SiKu please take a look this pr, thx. |
He must be enjoying the Dragon Boat Festival. Happy Dragon Boat Festival! |
hahah, just remind him to attention it~ |
@montaguelhz u can take a look issue #2352, u can implement it by the same way as this pr~(related sub-commands have implemented now) |
ok |
tools-v2/pkg/cli/command/curvebs/update/leader_schedule/leader_schedule.go
Outdated
Show resolved
Hide resolved
tools-v2/pkg/cli/command/curvebs/update/leader_schedule/leader_schedule.go
Outdated
Show resolved
Hide resolved
cicheck |
1 similar comment
cicheck |
return err.ToError() | ||
} | ||
out := make(map[string]string) | ||
if response, ok := result.(*schedule.RapidLeaderScheduleResponse); ok { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is ok
necessary here?
if response.GetStatusCode() != 0 { | ||
out[cobrautil.ROW_RESULT] = cobrautil.ROW_VALUE_FAILED | ||
out[cobrautil.ROW_REASON] = statuscode.ScheduleStatusCode_name[*response.StatusCode] | ||
} else { | ||
out[cobrautil.ROW_RESULT] = cobrautil.ROW_VALUE_SUCCESS | ||
out[cobrautil.ROW_REASON] = cobrautil.ROW_VALUE_NULL | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be handled by adding ErrBsRapidLeaderSchedule
.
if response.GetStatusCode() != 0 { | |
out[cobrautil.ROW_RESULT] = cobrautil.ROW_VALUE_FAILED | |
out[cobrautil.ROW_REASON] = statuscode.ScheduleStatusCode_name[*response.StatusCode] | |
} else { | |
out[cobrautil.ROW_RESULT] = cobrautil.ROW_VALUE_SUCCESS | |
out[cobrautil.ROW_REASON] = cobrautil.ROW_VALUE_NULL | |
} | |
err = cmderr.ErrBsRapidLeaderSchedule(response.GetStatusCode()) | |
if err.TypeCode() != cmderr.CODE_SUCCESS { | |
out[cobrautil.ROW_RESULT] = cobrautil.ROW_VALUE_FAILED | |
} else { | |
out[cobrautil.ROW_RESULT] = cobrautil.ROW_VALUE_SUCCESS | |
} | |
out[cobrautil.ROW_REASON] = err.Message |
lCmd.TableNew.Append(res) | ||
|
||
lCmd.Result = out | ||
lCmd.Error = cmderror.ErrSuccess() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lCmd.Error = cmderror.ErrSuccess() | |
lCmd.Error = err |
Signed-off-by: montaguelhz <[email protected]>
cicheck |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM~
cicheck |
1 similar comment
cicheck |
What problem does this PR solve?
Issue Number: #2359
Problem Summary:
What is changed and how it works?
What's Changed:
How it Works:
curve bs update leader-schedule --logicalpoolid 1
curve bs update leader-schedule-all
+---------+--------+
| RESULT | REASON |
+---------+--------+
| success | null |
+---------+--------+
+--------+--------------------+
| RESULT | REASON |
+--------+--------------------+
| failed | InvalidLogicalPool |
+--------+--------------------+
Side effects(Breaking backward compatibility? Performance regression?):
Check List