You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yingchun Lai (Code Review)" <ge...@cloudera.org> on 2019/03/22 11:10:11 UTC

[kudu-CR] [json] Add JsonFileReader to read JSON content from file

Yingchun Lai has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12835


Change subject: [json] Add JsonFileReader to read JSON content from file
......................................................................

[json] Add JsonFileReader to read JSON content from file

Introduce a new class JsonFileReader, which has the same APIs with
JsonReader. In fact they are derived from the same base class
JsonReaderBase, the difference is JsonReader reads JSON content from
raw string, while JsonFileReader reads content from file.

With JsonFileReader, it's able to read configuration from file, and
configfile reader is useful in some usecases. For example, a Kudu
administrator who manage several Kudu clusters is not easy to remember
all master rpc addresses when use kudu CLI tool to access them.
But it's easy to access these clusters by a simplfy cluster
name, which will be resolved to master rpc addresses by a JSON file.

I'll introduce a cluster name resolve method later.

Change-Id: I7c57c3c31189578268b126ef535bf3f933867288
---
M src/kudu/util/jsonreader-test.cc
M src/kudu/util/jsonreader.cc
M src/kudu/util/jsonreader.h
3 files changed, 300 insertions(+), 206 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/12835/1
-- 
To view, visit http://gerrit.cloudera.org:8080/12835
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7c57c3c31189578268b126ef535bf3f933867288
Gerrit-Change-Number: 12835
Gerrit-PatchSet: 1
Gerrit-Owner: Yingchun Lai <40...@qq.com>

[kudu-CR] [json] Add JsonFileReader to read JSON content from file

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has abandoned this change. ( http://gerrit.cloudera.org:8080/12835 )

Change subject: [json] Add JsonFileReader to read JSON content from file
......................................................................


Abandoned

We will use YAML reader instead.
-- 
To view, visit http://gerrit.cloudera.org:8080/12835
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I7c57c3c31189578268b126ef535bf3f933867288
Gerrit-Change-Number: 12835
Gerrit-PatchSet: 1
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [json] Add JsonFileReader to read JSON content from file

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/12835 )

Change subject: [json] Add JsonFileReader to read JSON content from file
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12835/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12835/1//COMMIT_MSG@14
PS1, Line 14: With JsonFileReader, it's able to read configuration from file, and
gflags natively supports "flags files" -- you can use --flagfile=/path/to/flags to load it. Is that sufficient for your use case?


http://gerrit.cloudera.org:8080/#/c/12835/1//COMMIT_MSG@19
PS1, Line 19: name, which will be resolved to master rpc addresses by a JSON file.
I think we should have a more general discussion about this direction. This has been a topic of debate in the past. Perhaps you can take over looking at https://issues.apache.org/jira/browse/KUDU-1948 ? The discussion there has a lot of good points brought up by Dan Burkert and others about some of the downsides of config files, so we should make sure we take them into account.

Maybe we can discuss a proposal there, and then once everyone agrees on the direction, we can look at the implementation?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7c57c3c31189578268b126ef535bf3f933867288
Gerrit-Change-Number: 12835
Gerrit-PatchSet: 1
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 22 Mar 2019 18:54:00 +0000
Gerrit-HasComments: Yes