Skip to content

feat:add bs query snapshot#2943

Open
ZackSoul wants to merge 1 commit intoopencurve:masterfrom
ZackSoul:query
Open

feat:add bs query snapshot#2943
ZackSoul wants to merge 1 commit intoopencurve:masterfrom
ZackSoul:query

Conversation

@ZackSoul
Copy link
Copy Markdown
Contributor

@ZackSoul ZackSoul commented Dec 4, 2023 •
edited
Loading

What problem does this PR solve?

Issue Number: #2582

Problem Summary:

What is changed and how it works?

add curve bs query snapshot

What's Changed:

How it Works:
query测试

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

Check List

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

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 4, 2023

cicheck

2 similar comments
@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 6, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 6, 2023

cicheck

caoxianfei1
caoxianfei1 reviewed Dec 12, 2023
View reviewed changes
Comment thread tools-v2/pkg/cli/command/curvebs/query/snapshot/snapshot.go Outdated
Comment on lines +142 to +151
row[cobrautil.ROW_STATUS] = fmt.Sprintf("%d", item.Status)
row[cobrautil.ROW_SNAPSHOT_SEQNUM] = fmt.Sprintf("%d", item.SeqNum)
row[cobrautil.ROW_FILE_LENGTH] = fmt.Sprintf("%d", item.FileLength)
row[cobrautil.ROW_PROGRESS] = fmt.Sprintf("%v", item.Progress)
row[cobrautil.ROW_CREATE_TIME] = time.Unix(int64(item.Time/1000000), 0).Format("2006-01-02 15:04:05")
Copy link
Copy Markdown

@caoxianfei1 caoxianfei1 Dec 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

convert int to string has better method?

caoxianfei1
caoxianfei1 approved these changes Dec 12, 2023
View reviewed changes
Copy link
Copy Markdown

@caoxianfei1 caoxianfei1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 12, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 13, 2023

cicheck

13 similar comments
@caoxianfei1
Copy link
Copy Markdown

caoxianfei1 commented Dec 14, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 14, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 14, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 14, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 14, 2023

cicheck

@caoxianfei1
Copy link
Copy Markdown

caoxianfei1 commented Dec 15, 2023

cicheck

@caoxianfei1
Copy link
Copy Markdown

caoxianfei1 commented Dec 15, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 16, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 17, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 17, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 17, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 17, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 19, 2023

cicheck

@caoxianfei1 caoxianfei1 reopened this Dec 19, 2023
@caoxianfei1
Copy link
Copy Markdown

caoxianfei1 commented Dec 19, 2023

cicheck

1 similar comment
@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 20, 2023

cicheck

@caoxianfei1 caoxianfei1 reopened this Dec 20, 2023
@caoxianfei1
Copy link
Copy Markdown

caoxianfei1 commented Dec 20, 2023

cicheck

1 similar comment
@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 20, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 26, 2023

cicheck

14 similar comments
@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 26, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 26, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 27, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 27, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 27, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 27, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 27, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 27, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 27, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 27, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 28, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 29, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Dec 29, 2023

cicheck

@montaguelhz
Copy link
Copy Markdown
Contributor

montaguelhz commented Jan 3, 2024

cicheck

montaguelhz
montaguelhz reviewed Jan 5, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@montaguelhz montaguelhz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good job

Comment thread tools-v2/README.md Outdated

```shell
curve bs query snapshot --path /test/test111 --snapshotstatus done
(other status: "in-progess" "deleting" "errorDeleteing" "canceling" "error")
Copy link
Copy Markdown
Contributor

@montaguelhz montaguelhz Jan 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe ## is better than ().

Copy link
Copy Markdown
Contributor Author

@ZackSoul ZackSoul Jan 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your comment, but I don't understand the ##, any example?

Copy link
Copy Markdown
Contributor

@montaguelhz montaguelhz Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for example,

curve bs query snapshot --path /test/test111 --snapshotstatus done # other status: "in-progess" "deleting" "errorDeleteing" "canceling" "error"

Copy link
Copy Markdown
Contributor

@montaguelhz montaguelhz Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like done is a keyword in shell, is that a problem？

Copy link
Copy Markdown
Contributor Author

@ZackSoul ZackSoul Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to be fine in the tests, and done should work only with for or while etc.

Comment thread tools-v2/pkg/cli/command/curvebs/query/snapshot/snapshot.go Outdated

const (
snapshotExample = `$ curve bs query snapshot --path /test/test111 --snapshotstatus done
(other status: "in-progress" "deleting" "errorDeleteing" "canceling" "error")`
Copy link
Copy Markdown
Contributor

@montaguelhz montaguelhz Jan 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto. And extra space will be shown if call help.

Copy link
Copy Markdown
Contributor Author

@ZackSoul ZackSoul Jan 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
Can I do like this to sovle the extar space?

Copy link
Copy Markdown
Contributor

@montaguelhz montaguelhz Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah

Copy link
Copy Markdown
Contributor Author

@ZackSoul ZackSoul Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PTAL, thanks@montaguelhz

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Jan 10, 2024

cicheck

2 similar comments
@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Jan 10, 2024

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Jan 10, 2024

cicheck

montaguelhz
montaguelhz reviewed Jan 11, 2024
View reviewed changes
Comment thread tools-v2/pkg/cli/command/curvebs/query/snapshot/snapshot.go Outdated

const (
snapshotExample = `$ curve bs query snapshot --path /test/test111 --snapshotstatus done
(other status: "in-progress" "deleting" "errorDeleteing" "canceling" "error")`
Copy link
Copy Markdown
Contributor

@montaguelhz montaguelhz Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, maybe I didn't make myself clear, please keep the README and the example consistent, all use #. this is the last suggestion.

Copy link
Copy Markdown
Contributor Author

@ZackSoul ZackSoul Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understand, that has been corrected.

@ZackSoul
feat(tool):add curve bs query snapshot
2ef8387 
Signed-off-by: ZackSoul <2269713951@qq.com>
@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Jan 11, 2024

cicheck

2 similar comments
@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Jan 11, 2024

cicheck

@montaguelhz
Copy link
Copy Markdown
Contributor

montaguelhz commented Jan 11, 2024

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Jan 11, 2024

cicheck

1 similar comment
@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Jan 11, 2024

cicheck

@montaguelhz
Copy link
Copy Markdown
Contributor

montaguelhz commented Jan 12, 2024

@caoxianfei1 it can be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@caoxianfei1 caoxianfei1 caoxianfei1 approved these changes

@montaguelhz montaguelhz montaguelhz approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ZackSoul @caoxianfei1 @montaguelhz