Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23224 )
Change subject: Add OpenAPI specification for REST API endpoints ...................................................................... Patch Set 9: (11 comments) A quick first pass. http://gerrit.cloudera.org:8080/#/c/23224/9/src/kudu/master/rest_catalog-test.cc File src/kudu/master/rest_catalog-test.cc: http://gerrit.cloudera.org:8080/#/c/23224/9/src/kudu/master/rest_catalog-test.cc@673 PS9, Line 673: VerifyObjectExists(tables_path, "get"); here and below: wrap this into NO_FATALS() to break out of the scenario right away if ASSER_TRUE() triggers inside of VerifyObjectExists? http://gerrit.cloudera.org:8080/#/c/23224/9/src/kudu/master/rest_catalog-test.cc@712 PS9, Line 712: ASSERT_STR_CONTAINS(content, "<html"); : ASSERT_STR_CONTAINS(content, "</html>"); : ASSERT_STR_CONTAINS(content, "<body"); : ASSERT_STR_CONTAINS(content, "</body>"); : ASSERT_STR_CONTAINS(content, "<head"); What are these for? This isn't helping to check for valid HTML, so why bother checking for the presence of both <html> and </html>? http://gerrit.cloudera.org:8080/#/c/23224/9/www/swagger/kudu-api.json File www/swagger/kudu-api.json: PS9: Does it possible to include information on security for all these documented methods, e.g. mention that sometimes it's necessary to authenticate to get access to any of the methods (i.e., in case of secured Kudu cluster), and that the SPNEGO is used for the authentication process? http://gerrit.cloudera.org:8080/#/c/23224/9/www/swagger/kudu-api.json@4 PS9, Line 4: Kudu REST API Do we call it 'Kudu REST API' as of now? I recall it was something about 'Kudu Master REST API' some time ago. http://gerrit.cloudera.org:8080/#/c/23224/9/www/swagger/kudu-api.json@14 PS9, Line 14: /tables For here and all other endpoints where authentication is applicable (I guess that's 100% or them): would it make sense to mention HTTP 401 status codes for non-authenticated access cases? I can see there are test cases for this least in changelist 06466b25e294b4d7d455e6c56263318dc34a80ef. Probably, that might be some common/blanket-style item. http://gerrit.cloudera.org:8080/#/c/23224/9/www/swagger/kudu-api.json@466 PS9, Line 466: "type": { : "type": "string", : "description": "Column data type", : "enum": [ : "INT8", "INT16", "INT32", "INT64", "UNIXTIME_MICROS", : "FLOAT", "DOUBLE", "DECIMAL", "STRING", "BOOL", "BINARY", : "VARCHAR", "DATE" : ] : }, As a part of KUDU-1261, support for one-dimensional arrays of scalar types has been introduced. It's not possible to specify a column of array type as of current version of RESTful API, correct? http://gerrit.cloudera.org:8080/#/c/23224/9/www/swagger/kudu-api.json@485 PS9, Line 485: "required": ["name", "type", "is_key", "is_nullable"] Kudu does have so-called auto-inrementing columns. Is it possible to create a table with auto-incrementing column using this RESTful API? http://gerrit.cloudera.org:8080/#/c/23224/9/www/swagger/kudu-api.json@495 PS9, Line 495: custom_hash_schema_ranges Where are these 'custom_hash_schema_ranges' defined in this API spec? I couldn't find them. http://gerrit.cloudera.org:8080/#/c/23224/9/www/swagger/kudu-api.json@546 PS9, Line 546: id What is 'id'? Is it at all applicable in this context when providing ColumnIdentifier for defining table schema if creating a table? http://gerrit.cloudera.org:8080/#/c/23224/9/www/swagger/kudu-api.json@562 PS9, Line 562: "type": "integer", : "minimum": 1, : "maximum": 7, : "default": 3, : "description": "Number of replicas for the table" nit: the indent seems to be wrong? Also, is it possible to express that even (i.e. non-odd) replication factors aren't allowed? BTW, 'minimum' and 'maximum' aren't something hard-coded, but are configuration settings and might change. Is it possible to reflect on this fact here? http://gerrit.cloudera.org:8080/#/c/23224/9/www/swagger/kudu-api.json@642 PS9, Line 642: "ADD_COLUMN", : "DROP_COLUMN", : "RENAME_COLUMN", : "ALTER_COLUMN", : "ADD_RANGE_PARTITION", : "DROP_RANGE_PARTITION" Don't Kudu master REST API support renaming tables? -- To view, visit http://gerrit.cloudera.org:8080/23224 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3981665c78f478e89d0300f3a2fc5d68b73b8762 Gerrit-Change-Number: 23224 Gerrit-PatchSet: 9 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, 28 Oct 2025 06:09:26 +0000 Gerrit-HasComments: Yes
