anujphadke has posted comments on this change.

Change subject: IMPALA-1850: Allow fs.defaultFS to be set to a non-HDFS 
filesystem
......................................................................


Patch Set 19:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/1121/19//COMMIT_MSG
Commit Message:

Line 7: IMPALA-1850 Allow fs.defaultFS to be set to a non-HDFS filesystem
> nit: : after 'IMPALA-1850'
Done


Line 13: Change-Id: I2f45bef6c94ece634045acb906d12591587ccfed
> nit: blank line before this one
Done


http://gerrit.cloudera.org:8080/#/c/1121/19/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:

Line 138:     "fs.defaultFS";
> one line
Done


Line 229:   if (FLAGS_use_statestore && statestore_port > 0) {
> can you add this blank line back?
Done


Line 413:   // get the fs.defaultFS value set in core-site.xml and assign it to
> nit: capitalise 'get'
Done


Line 424:   VLOG(1) << "Configured defaultFS:" << configured_defaultFs;
> LOG(INFO) is better here.
Done


http://gerrit.cloudera.org:8080/#/c/1121/19/be/src/util/hdfs-util.cc
File be/src/util/hdfs-util.cc:

Line 68:     return configured_defaultFs.compare(0,7,"hdfs://") == 0;
> spaces after each ','
Done


http://gerrit.cloudera.org:8080/#/c/1121/19/be/src/util/hdfs-util.h
File be/src/util/hdfs-util.h:

Line 51: static std::string configured_defaultFs;
> Move this to ExecEnv (where the variable is set), and add a getter to ExecE
Done


http://gerrit.cloudera.org:8080/#/c/1121/19/testdata/bin/generate-schema-statements.py
File testdata/bin/generate-schema-statements.py:

Line 198:  tblproperties["avro.schema.url"] = "%s/%s/%s/{table_name}.json" \
        :       % (DEFAULT_FS
> does any build use --hdfs_namenode, do you know? If not we can delete the o
I am not sure about this. How do I check this?


http://gerrit.cloudera.org:8080/#/c/1121/19/testdata/cluster/admin
File testdata/cluster/admin:

Line 53:   #TODO: Remove S3 from this condition once IMPALA-1850 goes in.
> remove this comment
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2f45bef6c94ece634045acb906d12591587ccfed
Gerrit-PatchSet: 19
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: anujphadke <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Casey Ching <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Henry Robinson <[email protected]>
Gerrit-Reviewer: Juan Yu <[email protected]>
Gerrit-Reviewer: Sailesh Mukil <[email protected]>
Gerrit-Reviewer: anujphadke <[email protected]>
Gerrit-HasComments: Yes

Reply via email to