Skip to content

Conversation

@bharatviswa504
Copy link
Contributor

@bharatviswa504 bharatviswa504 commented Jun 17, 2020

What changes were proposed in this pull request?

Remove the replay logic from actual requests.

What is the link to the Apache JIRA

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

How was this patch tested?

Existing tests, will add a few more tests to cover this scenario.

@bharatviswa504
Copy link
Contributor Author

Rebased patch with the latest master.

Copy link
Contributor

@hanishakoneru hanishakoneru left a comment

Choose a reason for hiding this comment

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

Thanks @bharat for working on this. Looks good overall.

I have very minor questions/ comments.
OMKeysDeleteResponse seems to have been missed in the cleanup.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should this return value be set to repeatedOmKeyInfo?

Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason for keeping this around?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Initially, the reason was because proto.lock file, if any breaking changes, it will fail to compile. I have removed this, as anyway HA is part of this release, and we can remove this field.

cc @avijayanhwx

Copy link
Contributor

Choose a reason for hiding this comment

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

The general guideline is breaking API changes are allowed pre-GA, but not after that.

@bharatviswa504
Copy link
Contributor Author

Thank You @hanishakoneru for the review.
OMKeysDeleteRequest will be fixed by HDDS-3930.

@hanishakoneru
Copy link
Contributor

Thanks @bharatviswa504 for working on this.
+1 pending CI.

@bharatviswa504 bharatviswa504 merged commit 90e8211 into apache:master Jul 14, 2020
@bharatviswa504
Copy link
Contributor Author

bharatviswa504 commented Jul 14, 2020

Thank You @hanishakoneru for the review and @avijayanhwx for the confirmation on proto changes.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants