[kudu-CR] Tweak ListMasters RPC error handling

[Dan Burkert (Code Review)]

Dan Burkert has submitted this change and it was merged.

Change subject: Tweak ListMasters RPC error handling
..


Tweak ListMasters RPC error handling

Unlike other master-specific RPCs, the ListMasters response contains an
AppStatusPB error type instead of a MasterErrorPB.  This makes it harder
to use ListMasters in a generic context with other master RPCs.

This commit changes the error type, while preserving the old type as a
deprecated field. There are no known users of the ListMasters RPC yet,
but maintaing backwards compat is still part of our contract.

Change-Id: I28e175df2cecad4f62806bc3ea0c314e5b861e40
Reviewed-on: http://gerrit.cloudera.org:8080/6704
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
3 files changed, 10 insertions(+), 6 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/6704
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I28e175df2cecad4f62806bc3ea0c314e5b861e40
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Tweak ListMasters RPC error handling

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Tweak ListMasters RPC error handling
..


Patch Set 2: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/6704
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I28e175df2cecad4f62806bc3ea0c314e5b861e40
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Tweak ListMasters RPC error handling

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: Tweak ListMasters RPC error handling
..


Patch Set 2:

We discussed testing offline and decided to punt.  A follow-up patch will be 
testing the success case.

-- 
To view, visit http://gerrit.cloudera.org:8080/6704
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I28e175df2cecad4f62806bc3ea0c314e5b861e40
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Tweak ListMasters RPC error handling

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Tweak ListMasters RPC error handling
..


Patch Set 2:

Maybe add a small unit test in master-test?

-- 
To view, visit http://gerrit.cloudera.org:8080/6704
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I28e175df2cecad4f62806bc3ea0c314e5b861e40
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Tweak ListMasters RPC error handling

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Tweak ListMasters RPC error handling
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6704/1/src/kudu/master/master_service.cc
File src/kudu/master/master_service.cc:

Line 365: 
resp->mutable_error()->mutable_status()->set_code(AppStatusPB::UNKNOWN_ERROR);
Doesn't this overwrite the work of StatusToPB()?


Line 370: 
resp->mutable_deprecated_error()->set_code(AppStatusPB::UNKNOWN_ERROR);
This too?


-- 
To view, visit http://gerrit.cloudera.org:8080/6704
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I28e175df2cecad4f62806bc3ea0c314e5b861e40
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Tweak ListMasters RPC error handling

[Dan Burkert (Code Review)]

Hello Adar Dembo,

I'd like you to do a code review.  Please visit

http://gerrit.cloudera.org:8080/6704

to review the following change.

Change subject: Tweak ListMasters RPC error handling
..

Tweak ListMasters RPC error handling

Unlike other master-specific RPCs, the ListMasters response contains an
AppStatusPB error type instead of a MasterErrorPB.  This makes it harder
to use ListMasters in a generic context with other master RPCs.

This commit changes the error type, while preserving the old type as a
deprecated field. There are no known users of the ListMasters RPC yet,
but maintaing backwards compat is still part of our contract.

Change-Id: I28e175df2cecad4f62806bc3ea0c314e5b861e40
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
3 files changed, 12 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/6704/1
-- 
To view, visit http://gerrit.cloudera.org:8080/6704
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I28e175df2cecad4f62806bc3ea0c314e5b861e40
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo