Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17444 )
Change subject: KUDU-3223: management of per-table limit by kudu CLI ...................................................................... Patch Set 15: Code-Review+1 (2 comments) LGTM, minus one small testing nit. http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/kudu-tool-test.cc@3975 PS15, Line 3975: KuduSchemaBuilder schema_builder; : schema_builder.AddColumn("key") : ->Type(client::KuduColumnSchema::INT32) : ->NotNull() : ->PrimaryKey(); : schema_builder.AddColumn("value") : ->Type(client::KuduColumnSchema::INT32) : ->NotNull(); : KuduSchema schema; : ASSERT_OK(schema_builder.Build(&schema)); : : // Create the table. : TestWorkload workload(cluster_.get()); : workload.set_table_name(kTableName); : workload.set_schema(schema); : workload.set_num_replicas(1); : workload.Setup(); nit: To avoid extraneous test code, we don't need to define a schema since the TestWorkload has one by default. Some other tests do since their test coverage relies on having specific schemas and columns, but these new tests don't. http://gerrit.cloudera.org:8080/#/c/17444/7/src/kudu/tools/tool_action_table.cc File src/kudu/tools/tool_action_table.cc: http://gerrit.cloudera.org:8080/#/c/17444/7/src/kudu/tools/tool_action_table.cc@1210 PS7, Line 1210: table_creator->dimension_label(table_req.dimension_label()); : } : return table_creator->Create(); : } : : : } // anonymous > BTW, what about using variadic arguments to read in the values? Also, it's Nice suggestion. This approach is much more elegant indeed. -- To view, visit http://gerrit.cloudera.org:8080/17444 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I59bb64cc85d5fca48ae8ec980f5cfc62e4b3a1d3 Gerrit-Change-Number: 17444 Gerrit-PatchSet: 15 Gerrit-Owner: Hongjiang Zhang <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Hongjiang Zhang <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 20 May 2021 21:21:02 +0000 Gerrit-HasComments: Yes
