[feat]tools-v2: add bs recover volume #2936

CrystalAnalyst · 2023-11-29T07:07:58Z

What problem does this PR solve?

Issue Number: #2588

Problem Summary: add recover as a middle command, and it has sub-command volume.

What is changed and how it works?

What's Changed: add recover middle command directory, and subcommand directory volume.

How it Works: recover.go is the middle command. Inside the sub-direcotry volume , the volume.go is the final command, while the recover.go wraps the RecoverFilePRC to execute the real recover procedure.

Side effects(Breaking backward compatibility? Performance regression?):

Check List

[yes] Relevant documentation/comments is changed or added
[yes] I acknowledge that all my contributions will be made under the project's license

baytan0720

Rebase commits as one. If you open a new pr, plz close the old one

baytan0720 · 2023-12-06T12:22:59Z

tools-v2/pkg/cli/command/curvebs/recover/volume/recover.go

Is this file unused?

the file is used, which wraps the RecoverFile RPC.

One volume.go file is ok.

One volume.go file is ok.

Actually, my first PR just did so, then I'm told that recover is a middle comamnd, maybe it would recover something else in the future? So I add this mid command. I think it's innocuous.

Yes, The recover is seen as middle command but you can refer the stop snapshot command.

Yes, The recover is seen as middle command but you can refer the stop snapshot command.

You're right! I finally got what you mean and make correspoding change. Please take a look, thanks.

Yes, but it also can be realized using one file and refer to other command.

Sorry, I don't catch what you mean. I just realize it using one file, Is there something still need to improve?

baytan0720 · 2023-12-18T05:54:29Z

@caoxianfei1 PTAL

caoxianfei1 · 2023-12-19T08:46:36Z

tools-v2/README.md

+---------+
+| RESULT  |
+---------+
+| success |
+---------+


maybe we can show path in table.

thanks for suggestion. done.

caoxianfei1 · 2023-12-19T08:51:41Z

tools-v2/pkg/cli/command/curvebs/recover/volume/recover.go

+func NewRecoverFileCommand() *RecoverFileCommand {
+	RecoverFileCommand := &RecoverFileCommand{
+		FinalCurveCmd: basecmd.FinalCurveCmd{},
+	}
+	basecmd.NewFinalCurveCli(&RecoverFileCommand.FinalCurveCmd, RecoverFileCommand)
+	return RecoverFileCommand
+}


put the func on top.

baytan0720 · 2023-12-24T17:14:35Z

cicheck

baytan0720 · 2023-12-25T02:19:57Z

cicheck

caoxianfei1 · 2023-12-25T02:53:48Z

@CrystalAnalyst need rebase otherwise ci may fail.

CrystalAnalyst · 2023-12-25T13:57:11Z

cicheck

CrystalAnalyst · 2023-12-25T15:11:56Z

cicheck

CrystalAnalyst · 2023-12-25T17:00:57Z

cicheck

caoxianfei1

LGTM!

caoxianfei1 · 2023-12-26T01:38:46Z

@CrystalAnalyst please fix the conflict.

montaguelhz

good job

montaguelhz · 2023-12-26T02:06:30Z

tools-v2/pkg/cli/command/curvebs/recover/volume/volume.go

+		return recoverCommand.Error.ToError()
+	}
+	out := make(map[string]string)
+	out[cobrautil.ROW_FILE_NAME] = *recoverCommand.Rpc.Request.FileName


it might be handled in Init

montaguelhz · 2023-12-26T02:10:15Z

tools-v2/pkg/cli/command/curvebs/recover/volume/volume.go

+
+var _ basecmd.FinalCurveCmdFunc = (*RecoverFileCommand)(nil)
+
+func (recoverCommand *RecoverFileCommand) Init(cmd *cobra.Command, args []string) error {


why not rCmd

montaguelhz · 2023-12-26T07:46:05Z

Use rebase instead of merge.

montaguelhz · 2023-12-26T07:49:07Z

Also, it might be better to leave a blank line between the comment and the code.

CrystalAnalyst · 2023-12-26T08:33:05Z

Also, it might be better to leave a blank line between the comment and the code.

okay.

CrystalAnalyst · 2023-12-26T08:34:23Z

Use rebase instead of merge.

yes, I reset to previous commit and use rebase solved the conflict justnow.

caoxianfei1 · 2023-12-26T08:40:35Z

the commit message suggestion is:
Feat/tool: add command recover volume

caoxianfei1 · 2023-12-26T08:40:46Z

cicheck

baytan0720

LGTM

montaguelhz · 2023-12-26T09:05:50Z

tools-v2/pkg/cli/command/curvebs/recover/volume/volume.go

+)
+
+const (
+	RecoverExample = `curve bs recover volume --path /curvebs-volume-path 


const ( RecoverExample = `curve bs recover volume --path /curvebs-volume-path --user username [--password password] [--fileid fileid]` )

Extra Spaces may appear when executing help command.

CrystalAnalyst · 2023-12-26T12:21:01Z

cicheck

Signed-off-by: kevin <[email protected]>

baytan0720 · 2023-12-26T13:59:15Z

cicheck

caoxianfei1 · 2023-12-27T02:23:12Z

LGTM!

montaguelhz

LGTM

CrystalAnalyst · 2024-01-03T07:13:33Z

cicheck

CrystalAnalyst · 2024-01-03T13:30:45Z

All checks have passed. Is there anyone who is authorized to help me merge this pull request ? @caoxianfei1 @montaguelhz @baytan0720 I'd appreciate that, thanks.

CrystalAnalyst force-pushed the feature-recover branch from a193cc0 to bb2857d Compare November 29, 2023 07:11

baytan0720 reviewed Dec 6, 2023

View reviewed changes

baytan0720 requested a review from Cyber-SiKu December 6, 2023 12:25

baytan0720 changed the title ~~Feat: add curve bs recover volume~~ [feat]tools-v2: add bs recover volume Dec 6, 2023

CrystalAnalyst force-pushed the feature-recover branch 2 times, most recently from 927d463 to 48b6138 Compare December 6, 2023 15:06

baytan0720 requested a review from caoxianfei1 December 18, 2023 05:54

caoxianfei1 reviewed Dec 19, 2023

View reviewed changes

CrystalAnalyst force-pushed the feature-recover branch 2 times, most recently from 9ce7daf to 5a4c6bd Compare December 19, 2023 13:32

CrystalAnalyst requested review from caoxianfei1 and baytan0720 December 19, 2023 13:43

CrystalAnalyst force-pushed the feature-recover branch 6 times, most recently from d34076c to 89f1287 Compare December 23, 2023 13:23

CrystalAnalyst force-pushed the feature-recover branch 2 times, most recently from f95d183 to 7d4dafd Compare December 25, 2023 10:38

caoxianfei1 approved these changes Dec 26, 2023

View reviewed changes

montaguelhz reviewed Dec 26, 2023

View reviewed changes

CrystalAnalyst force-pushed the feature-recover branch from 7d4dafd to e24428d Compare December 26, 2023 07:38

CrystalAnalyst force-pushed the feature-recover branch 3 times, most recently from 4ef46ea to 328d977 Compare December 26, 2023 08:31

CrystalAnalyst force-pushed the feature-recover branch from 328d977 to ef54fb6 Compare December 26, 2023 08:37

baytan0720 approved these changes Dec 26, 2023

View reviewed changes

montaguelhz reviewed Dec 26, 2023

View reviewed changes

[feat]:add command bs recover volume

3491478

Signed-off-by: kevin <[email protected]>

CrystalAnalyst force-pushed the feature-recover branch from ef54fb6 to 3491478 Compare December 26, 2023 13:54

caoxianfei1 approved these changes Dec 27, 2023

View reviewed changes

montaguelhz approved these changes Dec 27, 2023

View reviewed changes

caoxianfei1 merged commit 3a82cb7 into opencurve:master Jan 4, 2024
4 checks passed


		var _ basecmd.FinalCurveCmdFunc = (*RecoverFileCommand)(nil)

		func (recoverCommand RecoverFileCommand) Init(cmd cobra.Command, args []string) error {

[feat]tools-v2: add bs recover volume #2936

[feat]tools-v2: add bs recover volume #2936

Conversation

CrystalAnalyst commented Nov 29, 2023

What problem does this PR solve?

What is changed and how it works?

Check List

baytan0720 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

baytan0720 commented Dec 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

baytan0720 commented Dec 24, 2023

baytan0720 commented Dec 25, 2023

caoxianfei1 commented Dec 25, 2023 • edited Loading

CrystalAnalyst commented Dec 25, 2023

CrystalAnalyst commented Dec 25, 2023

CrystalAnalyst commented Dec 25, 2023

caoxianfei1 left a comment

Choose a reason for hiding this comment

caoxianfei1 commented Dec 26, 2023

montaguelhz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

montaguelhz commented Dec 26, 2023

montaguelhz commented Dec 26, 2023

CrystalAnalyst commented Dec 26, 2023

CrystalAnalyst commented Dec 26, 2023

caoxianfei1 commented Dec 26, 2023

caoxianfei1 commented Dec 26, 2023

baytan0720 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CrystalAnalyst commented Dec 26, 2023

baytan0720 commented Dec 26, 2023

caoxianfei1 commented Dec 27, 2023

montaguelhz left a comment

Choose a reason for hiding this comment

CrystalAnalyst commented Jan 3, 2024

CrystalAnalyst commented Jan 3, 2024

caoxianfei1 commented Dec 25, 2023 •

edited

Loading