Gabriella Lotz has posted comments on this change. ( http://gerrit.cloudera.org:8080/22744 )
Change subject: KUDU-3608 REST API Follow-up ...................................................................... Patch Set 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/22744/3/src/kudu/master/rest_catalog-test.cc File src/kudu/master/rest_catalog-test.cc: http://gerrit.cloudera.org:8080/#/c/22744/3/src/kudu/master/rest_catalog-test.cc@234 PS3, Line 234: ASSERT_TRUE(!s.ok()); > Here and elsewhere: consider checking for exact type of non-OK status expec For REST API errors, any non-200 HTTP status code results in a RemoteError, not a more specific status like NotFound or InvalidArgument. This is because the EasyCurl::FetchURL() helper wraps all HTTP error responses into Status::RemoteError. Therefore, inspecting the HTTP code or message string (e.g., "HTTP 404") is the correct way to validate expected errors in this context. http://gerrit.cloudera.org:8080/#/c/22744/3/src/kudu/master/rest_catalog_path_handlers.cc File src/kudu/master/rest_catalog_path_handlers.cc: http://gerrit.cloudera.org:8080/#/c/22744/3/src/kudu/master/rest_catalog_path_handlers.cc@90 PS3, Line 90: } : if (status.IsInvalidArgument() || status.IsA > Consider using find() instead of double lookup. Done http://gerrit.cloudera.org:8080/#/c/22744/3/src/kudu/master/rest_catalog_path_handlers.cc@197 PS3, Line 197: void RestCatalogPathHandlers::HandlePostTables(ostringstream* output, : const Webserver::WebRequest& req, : HttpStatusCode* status_code) { : CreateTableRequestPB request; : C > IIUC, CreateTableWithUser() may return Status::NotAuthorized() and Status: Done http://gerrit.cloudera.org:8080/#/c/22744/3/src/kudu/master/rest_catalog_test_base.h File src/kudu/master/rest_catalog_test_base.h: http://gerrit.cloudera.org:8080/#/c/22744/3/src/kudu/master/rest_catalog_test_base.h@54 PS3, Line 54: ring> colum > nit for here and elsewhere: the Kudu project doesn't use the "camelCase" fo Done http://gerrit.cloudera.org:8080/#/c/22744/3/src/kudu/master/rest_catalog_test_base.h@55 PS3, Line 55: > nit: these two lines might have become just Done -- To view, visit http://gerrit.cloudera.org:8080/22744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3cdb14d5a932eb8d1ba209ca803731749117024e Gerrit-Change-Number: 22744 Gerrit-PatchSet: 4 Gerrit-Owner: Gabriella Lotz <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Gabriella Lotz <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Tue, 22 Apr 2025 09:58:24 +0000 Gerrit-HasComments: Yes
