Skip to content

Conversation

@xBis7
Copy link
Contributor

@xBis7 xBis7 commented Jun 2, 2023

What changes were proposed in this pull request?

This patch adds a new CLI command for listing SnapshotDiff jobs for a bucket based on jobStatus. There is also the option to list all available jobs for the bucket.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-8704

How was this patch tested?

This patch was tested with new tests. Also, snapshot-sh.robot acceptance test was expanded to include listing snapshot diff jobs.

@xBis7
Copy link
Contributor Author

xBis7 commented Jun 2, 2023

@hemantk-12 @smengcl Can you take a look at this PR?

@adoroszlai adoroszlai added the snapshot https://issues.apache.org/jira/browse/HDDS-6517 label Jun 2, 2023
@smengcl smengcl changed the title HDDS-8704. Snapshot: List snapdiff job HDDS-8704. [Snapshot] Add CLI to list SnapDiff jobs Jun 7, 2023
@smengcl smengcl added the gr label Jun 7, 2023
Copy link
Contributor

@hemantk-12 hemantk-12 left a comment

Choose a reason for hiding this comment

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

Thanks for the patch @xBis7.

Left some comments.

@xBis7
Copy link
Contributor Author

xBis7 commented Jun 9, 2023

@hemantk-12 I've addressed all your comments. The only ones left are about implementing pagination and renaming SnapshotDiffJob to SnapshotDiffJobInfo. These two can be addressed in separate PRs.

I've also added a small test for validating error responses due to invalid volume name or bucket name or snapshot path.

@hemantk-12
Copy link
Contributor

@hemantk-12 I've addressed all your comments. The only ones left are about implementing pagination and renaming SnapshotDiffJob to SnapshotDiffJobInfo. These two can be addressed in separate PRs.

I've also added a small test for validating error responses due to invalid volume name or bucket name or snapshot path.

Thanks for adding integration test for invalid volume and bucket names.

@xBis7
Copy link
Contributor Author

xBis7 commented Jun 13, 2023

@hemantk-12 I've addressed all your comments and also merged the branch into master and rewrote the tests to use the new TestSnapshotDiffManager setup.

@smengcl
Copy link
Contributor

smengcl commented Jun 21, 2023

Hi @xBis7 , looks like this PR has a conflict with HDDS-8490 (PR #4819 ) which is merged just now. Would you merge latest master into this PR to resolve the conflicts? Thx

@xBis7
Copy link
Contributor Author

xBis7 commented Jun 21, 2023

Hey @smengcl, I've resolved the conflicts and merged the branch into master.

Copy link
Contributor

@prashantpogde prashantpogde left a comment

Choose a reason for hiding this comment

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

thanks @xBis7 .The changes look good to me.

@prashantpogde prashantpogde merged commit 6035a8b into apache:master Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

snapshot https://issues.apache.org/jira/browse/HDDS-6517

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants