-
Notifications
You must be signed in to change notification settings - Fork 279
rbd: add rbd_snap_remove_by_id #1187
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
Conversation
|
This pull request now has conflicts with the target branch. |
f49a7d1 to
5fbce6b
Compare
|
Thanks @anoopcs9 and @phlogistonjohn! Fixed the test-case and solved the merge conflicts in the api-status. |
anoopcs9
left a comment
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.
Probably replace librbd:rbd_snap_remove_by_id in the commit message (Add librbd:rbd_snap_remove_by_id to the API status) to RemoveSnapByID to reflect the new go-ceph API than the underlying C API?
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" |
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.
I think, for this test case, we only need either assert or require.
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.
I don't see using both as an issue: assert sets the test as failed but keeps going; require stops the test then and there if it fails. both have their uses. but I'm not objecting to simplifying it either.
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" |
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.
I don't see using both as an issue: assert sets the test as failed but keeps going; require stops the test then and there if it fails. both have their uses. but I'm not objecting to simplifying it either.
5fbce6b to
3235909
Compare
@anoopcs9, I've updated the commit message. Please have a look again, thanks! |
Pull request has been modified.
phlogistonjohn
left a comment
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
|
@Mergifyio rebase |
Image.RemoveSnapByID removes an existing snapshot. This can be used to manually remove a snapshot from the trash. Signed-off-by: Niels de Vos <ndevos@ibm.com>
The librbd:rbd_snap_remove_by_id function is implemented by Image.RemoveSnapByID inside the rbd package. Signed-off-by: Niels de Vos <ndevos@ibm.com>
✅ Branch has been successfully rebased |
3235909 to
137ff3d
Compare
Image.RemoveSnapByID removes an existing snapshot. This can be used to
manually remove a snapshot from the trash.
Closes: #1183