Gabriella Lotz has posted comments on this change. ( http://gerrit.cloudera.org:8080/21823 )
Change subject: [WIP] KUDU-3608: REST API for Metadata Management ...................................................................... Patch Set 24: (10 comments) http://gerrit.cloudera.org:8080/#/c/21823/23//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21823/23//COMMIT_MSG@26 PS23, Line 26: Deleting, altering, or retrieving a table object requires : the table's ID. Operations using table names are n > You can remove this line, once the kdc tests are included. Done http://gerrit.cloudera.org:8080/#/c/21823/23//COMMIT_MSG@38 PS23, Line 38: - Adding ranger itests > You can mention at the end that there will be follow-up patches for: Done http://gerrit.cloudera.org:8080/#/c/21823/23/src/kudu/master/master.cc File src/kudu/master/master.cc: http://gerrit.cloudera.org:8080/#/c/21823/23/src/kudu/master/master.cc@135 PS23, Line 135: > "Enables REST API endpoints for metadata management in the master server." Done http://gerrit.cloudera.org:8080/#/c/21823/23/src/kudu/master/master.cc@136 PS23, Line 136: false, > Since SPNEGO works now with the api, i think we can remove this line, right Done http://gerrit.cloudera.org:8080/#/c/21823/23/src/kudu/master/master.cc@137 PS23, Line 137: "Enables REST API endpoints in the master server. The flag webserver_enabled must be " > Looking at webserver_enabled flag, if that uses Done http://gerrit.cloudera.org:8080/#/c/21823/18/src/kudu/master/master_path_handlers.cc File src/kudu/master/master_path_handlers.cc: http://gerrit.cloudera.org:8080/#/c/21823/18/src/kudu/master/master_path_handlers.cc@82 PS18, Line 82: using kudu::security::DataFormat; > add #undef RETURN_JSON_ERROR at the end of the file so that this macro is o Done http://gerrit.cloudera.org:8080/#/c/21823/18/src/kudu/master/rest_catalog-test.cc File src/kudu/master/rest_catalog-test.cc: http://gerrit.cloudera.org:8080/#/c/21823/18/src/kudu/master/rest_catalog-test.cc@112 PS18, Line 112: if (!s.ok()) { > these strings can be const as well Done http://gerrit.cloudera.org:8080/#/c/21823/23/src/kudu/master/rest_catalog-test.cc File src/kudu/master/rest_catalog-test.cc: http://gerrit.cloudera.org:8080/#/c/21823/23/src/kudu/master/rest_catalog-test.cc@1 PS23, Line 1: // Licensed to the Apache Software Foundation (ASF) under one > Please add the Apache license in the beginning of this files as well. Done http://gerrit.cloudera.org:8080/#/c/21823/23/src/kudu/master/rest_catalog-test.cc@106 PS23, Line 106: return s; : } : : string GetTableId(const string& table_name) { : shared_ptr<KuduTable> table; : Status s = client_->OpenTable(table_name, &table); : if (!s.ok()) { : LOG(ERROR) << "OpenTable failed: " << s.ToString(); : return ""; : } : if (table->name() == table_name) { : return table->id(); : } : return ""; : } > nit: I just saw that we have "class KuduRegex " in src/kudu/util/regex.h. Done http://gerrit.cloudera.org:8080/#/c/21823/23/src/kudu/master/rest_catalog_path_handlers.cc File src/kudu/master/rest_catalog_path_handlers.cc: http://gerrit.cloudera.org:8080/#/c/21823/23/src/kudu/master/rest_catalog_path_handlers.cc@58 PS23, Line 58: RestCatalogPathHandlers::~RestCatalogPathHandlers() {} : : void RestCatalogPathHandlers::HandleApiTableEndpoint(const Webserver::WebRequest& req, : Webserver::PrerenderedWebResponse* resp) { : string table_id = req.path_params.at("table_id"); : ostringstream* output = &resp->output; : JsonWriter jw(output, JsonWriter::COMPACT); : : if (table_id.length() != 32) { : RETURN_JSON_ERROR(jw, "Invalid table ID", resp->status_code, HttpStatusCode::BadRequest); : } : CatalogManager::ScopedLeaderSharedLock l(master_->catalog_manager()); : scoped_refptr<TableInfo> table; : Status status = master_->catalog_manager()->GetTableInfo(table_id, &table); : : if (!status.ok()) { > I dont see this part being used in this file. Can we remove it? Done -- To view, visit http://gerrit.cloudera.org:8080/21823 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I67f964c4f950edfde31772cafd5c3ed5d6b87413 Gerrit-Change-Number: 21823 Gerrit-PatchSet: 24 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: Thu, 16 Jan 2025 18:17:06 +0000 Gerrit-HasComments: Yes
