Attila Bukor has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17555 )

Change subject: [rest] add rest implementation
......................................................................


Patch Set 42:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/17555/42/src/kudu/rest/controller.cc
File src/kudu/rest/controller.cc:

PS42:
can you add some simple unit tests? it would make it easier to test, especially 
without the integration tests later on in the chain.


http://gerrit.cloudera.org:8080/#/c/17555/42/src/kudu/rest/controller.cc@106
PS42, Line 106:   KuduSchema schema = CreateSchema(kudu_table_schema_dto, 
part_col);
> You're seeing this issue because the "using kudu::client::sp::shared_ptr" d
I think this was my suggestion actually, CreateSchema() modifies the vector 
passed to it, so it needs to be a pointer. I guess it's okay for it to be a raw 
pointer though.


http://gerrit.cloudera.org:8080/#/c/17555/42/src/kudu/rest/rest_server.cc
File src/kudu/rest/rest_server.cc:

http://gerrit.cloudera.org:8080/#/c/17555/42/src/kudu/rest/rest_server.cc@73
PS42, Line 73:       .setVersion("1.15.0")
Can you change it to set the version number dynamically?



--
To view, visit http://gerrit.cloudera.org:8080/17555
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2ca3121fd7e95a1267853be45cb5f5855298c763
Gerrit-Change-Number: 17555
Gerrit-PatchSet: 42
Gerrit-Owner: Khazar Mammadli <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 04 Aug 2021 14:24:19 +0000
Gerrit-HasComments: Yes

Reply via email to