You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2018/05/11 18:45:45 UTC
[kudu-CR] catalog-manager: refactor error handling
Hello Adar Dembo, Hao Hao,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/10376
to review the following change.
Change subject: catalog-manager: refactor error handling
......................................................................
catalog-manager: refactor error handling
This commit refactors how the SetupError helper works slightly in order
to reduce the prevelance of temporary Status locals. I've found the
Status locals to be error prone in the past due to shadowing.
Change-Id: I695a62a75a3b1979741e2d7105ed131eace64b6d
---
M src/kudu/master/catalog_manager.cc
1 file changed, 110 insertions(+), 134 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/10376/1
--
To view, visit http://gerrit.cloudera.org:8080/10376
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I695a62a75a3b1979741e2d7105ed131eace64b6d
Gerrit-Change-Number: 10376
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
[kudu-CR] catalog-manager: refactor error handling
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10376 )
Change subject: catalog-manager: refactor error handling
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10376
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I695a62a75a3b1979741e2d7105ed131eace64b6d
Gerrit-Change-Number: 10376
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 14 May 2018 17:40:57 +0000
Gerrit-HasComments: No
[kudu-CR] catalog-manager: refactor error handling
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10376
to look at the new patch set (#2).
Change subject: catalog-manager: refactor error handling
......................................................................
catalog-manager: refactor error handling
This commit refactors how the SetupError helper works slightly in order
to reduce the prevelance of temporary Status locals. I've found the
Status locals to be error prone in the past due to shadowing.
Change-Id: I695a62a75a3b1979741e2d7105ed131eace64b6d
---
M src/kudu/client/client-test.cc
M src/kudu/master/catalog_manager.cc
2 files changed, 117 insertions(+), 141 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/10376/2
--
To view, visit http://gerrit.cloudera.org:8080/10376
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I695a62a75a3b1979741e2d7105ed131eace64b6d
Gerrit-Change-Number: 10376
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] catalog-manager: refactor error handling
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10376 )
Change subject: catalog-manager: refactor error handling
......................................................................
catalog-manager: refactor error handling
This commit refactors how the SetupError helper works slightly in order
to reduce the prevelance of temporary Status locals. I've found the
Status locals to be error prone in the past due to shadowing.
Change-Id: I695a62a75a3b1979741e2d7105ed131eace64b6d
Reviewed-on: http://gerrit.cloudera.org:8080/10376
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/client/client-test.cc
M src/kudu/master/catalog_manager.cc
2 files changed, 117 insertions(+), 141 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/10376
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I695a62a75a3b1979741e2d7105ed131eace64b6d
Gerrit-Change-Number: 10376
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] catalog-manager: refactor error handling
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10376 )
Change subject: catalog-manager: refactor error handling
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10376
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I695a62a75a3b1979741e2d7105ed131eace64b6d
Gerrit-Change-Number: 10376
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 11 May 2018 18:59:26 +0000
Gerrit-HasComments: No
[kudu-CR] catalog-manager: refactor error handling
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/10376 )
Change subject: catalog-manager: refactor error handling
......................................................................
Patch Set 1: Code-Review+1
(2 comments)
LGTM, just some minor nits.
http://gerrit.cloudera.org:8080/#/c/10376/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/10376/1/src/kudu/master/catalog_manager.cc@2149
PS1, Line 2149: T
nit: table (lower case)
http://gerrit.cloudera.org:8080/#/c/10376/1/src/kudu/master/catalog_manager.cc@4502
PS1, Line 4502: T
nit: the table does not exist.
--
To view, visit http://gerrit.cloudera.org:8080/10376
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I695a62a75a3b1979741e2d7105ed131eace64b6d
Gerrit-Change-Number: 10376
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 11 May 2018 21:46:26 +0000
Gerrit-HasComments: Yes