feature: add bs delete snapshot #2889
Conversation
|
cicheck |
2 similar comments
|
cicheck |
|
cicheck |
|
|
||
| func (sCmd *SnapShotCommand) Init(cmd *cobra.Command, args []string) error { | ||
| snapshotAddrs, err := config.GetBsSnapshotAddrSlice(sCmd.Cmd) | ||
| if err.TypeCode() != cmderror.CODE_SUCCESS || len(snapshotAddrs) == 0 { |
There was a problem hiding this comment.
The two cases are diff, maybe they are not suitable for processing in a statement.
| snapshotutil.QueryOffset: snapshotutil.Offset, | ||
| } | ||
| if sCmd.failed { | ||
| params[snapshotutil.QueryStatus] = 5 |
There was a problem hiding this comment.
It might be better to change the hard code.
There was a problem hiding this comment.
What does it mean to change the hard code？
There was a problem hiding this comment.
For example, you can use static variable to record what 5 means.
There was a problem hiding this comment.
ok，understand，I will modify it.
| if sCmd.failed { | ||
| params[snapshotutil.QueryStatus] = 5 | ||
| } | ||
| if sCmd.uuid != "*" { |
|
cicheck |
6 similar comments
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
tools-v2/README.md
Outdated
| Usage: | ||
|
|
||
| ```shell | ||
| curve bs delete snapshot |
There was a problem hiding this comment.
Add example of optional parameters
| ) | ||
|
|
||
| const ( | ||
| snapshotExample = `$ curve bs delete snapshot` |
| func (sCmd *SnapShotCommand) Init(cmd *cobra.Command, args []string) error { | ||
| snapshotAddrs, err := config.GetBsSnapshotAddrSlice(sCmd.Cmd) | ||
| if err.TypeCode() != cmderror.CODE_SUCCESS { | ||
| return err.ToError() |
There was a problem hiding this comment.
set a value for sCmd.Error
There was a problem hiding this comment.
Do you mean setting value to this err.ToError()?
|
|
|
cicheck |
1 similar comment
|
cicheck |
|
@Cyber-SiKu PTAL~ |
|
However, if you want to merge the pr to master, please merge your commits into one. |
ZackSoul
force-pushed
the
bs-delete-snapshot
branch
2 times, most recently
from
ac67d2f to
bcbe08f
Compare
|
cicheck |
| rows := make([]map[string]string, 0) | ||
| for _, snapshot := range snapshotsList { | ||
| row := make(map[string]string) | ||
| err := DeleteSnapShot(sCmd.snapshotAddrs, sCmd.timeout, snapshot) |
There was a problem hiding this comment.
should handle error here?
tools-v2/pkg/config/bs.go
Outdated
| @@ -675,6 +680,10 @@ func AddBsSnapshotIDOptionFlag(cmd *cobra.Command) { | |||
| AddBsStringOptionFlag(cmd, CURVEBS_SNAPSHOT_ID, "snapshot seqId") | |||
| } | |||
|
|
|||
| func AddBsSnapshotFailedDOptionFlag(cmd *cobra.Command) { | |||
There was a problem hiding this comment.
AddBsSnapshotFailedOptionFlag?
ZackSoul
force-pushed
the
bs-delete-snapshot
branch
2 times, most recently
from
691c708 to
d2bfa24
Compare
|
cicheck |
1 similar comment
|
cicheck |
ZackSoul
force-pushed
the
bs-delete-snapshot
branch
2 times, most recently
from
0ed3edc to
e22656f
Compare
|
cicheck |
27 similar comments
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
What problem does this PR solve?
Issue Number: #2578
Problem Summary:
What is changed and how it works?
What's Changed:
add bs delete snapshot
How it Works:
Side effects(Breaking backward compatibility? Performance regression?):
Check List