You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "John Russell (Code Review)" <ge...@cloudera.org> on 2017/01/10 07:31:03 UTC

[Impala-ASF-CR] Patch references to Cloudera and CDH in Impala tutorial

John Russell has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/5663

Change subject: Patch references to Cloudera and CDH in Impala tutorial
......................................................................

Patch references to Cloudera and CDH in Impala tutorial

There was one tutorial that actually ran under the
'cloudera' user and so repeated that name over and
over in directory and HDFS paths. I switched that to
'username'.

I suppressed some <note> and <li> tags with Cloudera
Manager-specific details. Will physically remove those
from the source in a subsequent iteration.

I left several instances of audience="Cloudera" because
those will be changed to audience="hidden" as part of
a separate change request.

I marked with rev="upstream" some <codeblock> tags
containing impala-shell banners with a Cloudera copyright
statement. Will decide on a convention to handle those
(elide those lines, or use a conref to consistently
substitute the generic equivalent) and do that in a
followup patch set.

Change-Id: I44245b65ce6f247ae8771f582f4b33c3712145ae
---
M docs/topics/impala_tutorial.xml
1 file changed, 24 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/5663/1
-- 
To view, visit http://gerrit.cloudera.org:8080/5663
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I44245b65ce6f247ae8771f582f4b33c3712145ae
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jr...@cloudera.com>

[Impala-ASF-CR] Patch references to Cloudera and CDH in Impala tutorial

Posted by "John Russell (Code Review)" <ge...@cloudera.org>.
Hello Laurel Hale,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/5663

to look at the new patch set (#3).

Change subject: Patch references to Cloudera and CDH in Impala tutorial
......................................................................

Patch references to Cloudera and CDH in Impala tutorial

There was one tutorial that actually ran under the
'cloudera' user and so repeated that name over and
over in directory and HDFS paths. I switched that to
'username'.

I suppressed some <note> and <li> tags with Cloudera
Manager-specific details. Will physically remove those
from the source in a subsequent iteration.

I left several instances of audience="Cloudera" because
those will be changed to audience="hidden" as part of
a separate change request.

I marked with rev="upstream" some <codeblock> tags
containing impala-shell banners with a Cloudera copyright
statement. Will decide on a convention to handle those
(elide those lines, or use a conref to consistently
substitute the generic equivalent) and do that in a
followup patch set.

Change-Id: I44245b65ce6f247ae8771f582f4b33c3712145ae
---
M docs/impala_keydefs.ditamap
M docs/topics/impala_tutorial.xml
2 files changed, 27 insertions(+), 27 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/5663/3
-- 
To view, visit http://gerrit.cloudera.org:8080/5663
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I44245b65ce6f247ae8771f582f4b33c3712145ae
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>

[Impala-ASF-CR] Patch references to Cloudera and CDH in Impala tutorial

Posted by "Laurel Hale (Code Review)" <ge...@cloudera.org>.
Laurel Hale has posted comments on this change.

Change subject: Patch references to Cloudera and CDH in Impala tutorial
......................................................................


Patch Set 1: Code-Review-1

(2 comments)

The use of keyref to change "CDH" to a Hadoop distribution number didn't seem to work (see line 60) and use of parentheses not necessary in line 1700.

http://gerrit.cloudera.org:8080/#/c/5663/1/docs/topics/impala_tutorial.xml
File docs/topics/impala_tutorial.xml:

Line 60:         If you already have a <keyword keyref="hadoop_distro"/> environment set up and just need to add Impala to it, follow the installation
This doesn't seem to work. It's showing up as "CDH" in the build.


PS1, Line 1700: (Currently, this technique only works for Parquet files.)
Parentheses not necessary.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I44245b65ce6f247ae8771f582f4b33c3712145ae
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Patch references to Cloudera and CDH in Impala tutorial

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: Patch references to Cloudera and CDH in Impala tutorial
......................................................................


Patch Set 1:

(2 comments)

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

Line 22: I marked with rev="upstream" some <codeblock> tags
I don't see upstream in any ditaval file. Where is it defined?


http://gerrit.cloudera.org:8080/#/c/5663/1/docs/topics/impala_tutorial.xml
File docs/topics/impala_tutorial.xml:

PS1, Line 698: from the sample TPC-DS kit for Impala.
The only one of these is Cloudera's and it was very broken last I checked.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I44245b65ce6f247ae8771f582f4b33c3712145ae
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Patch references to Cloudera and CDH in Impala tutorial

Posted by "Laurel Hale (Code Review)" <ge...@cloudera.org>.
Laurel Hale has posted comments on this change.

Change subject: Patch references to Cloudera and CDH in Impala tutorial
......................................................................


Patch Set 3: Code-Review+1

Looks good.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I44245b65ce6f247ae8771f582f4b33c3712145ae
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] Patch references to Cloudera and CDH in Impala tutorial

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: Patch references to Cloudera and CDH in Impala tutorial
......................................................................


Patch Set 2:

Laurel, if this addresses your -1, I am ready to +2.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I44245b65ce6f247ae8771f582f4b33c3712145ae
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] Patch references to Cloudera and CDH in Impala tutorial

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: Patch references to Cloudera and CDH in Impala tutorial
......................................................................


Patch Set 3: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I44245b65ce6f247ae8771f582f4b33c3712145ae
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] Patch references to Cloudera and CDH in Impala tutorial

Posted by "Laurel Hale (Code Review)" <ge...@cloudera.org>.
Laurel Hale has posted comments on this change.

Change subject: Patch references to Cloudera and CDH in Impala tutorial
......................................................................


Patch Set 2: Code-Review-1

(1 comment)

Change the article before the <hadoop_distro> keydef and you're golden. The value that replaces <hadoop_distro> is "Apache Hadoop" and therefore requires the article "an" instead of "a."

http://gerrit.cloudera.org:8080/#/c/5663/2/docs/topics/impala_tutorial.xml
File docs/topics/impala_tutorial.xml:

PS2, Line 60: a
This should be "an" since the value that replaces "hadoop_distro" begins with the word, "Apache," a vowel. Sorry. :-(


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I44245b65ce6f247ae8771f582f4b33c3712145ae
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Patch references to Cloudera and CDH in Impala tutorial

Posted by "John Russell (Code Review)" <ge...@cloudera.org>.
John Russell has posted comments on this change.

Change subject: Patch references to Cloudera and CDH in Impala tutorial
......................................................................


Patch Set 2:

(4 comments)

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

Line 22: I marked with rev="upstream" some <codeblock> tags
> I don't see upstream in any ditaval file. Where is it defined?
It's a purely user-defined attribute that we use for bookkeeping purposes, e.g. we use rev="IMPALA-abcd" to indicate the relevant JIRA associated with a change, and rev="upstream" to mark spots requiring future cleanup as part of the Apache cleanup.


http://gerrit.cloudera.org:8080/#/c/5663/1/docs/topics/impala_tutorial.xml
File docs/topics/impala_tutorial.xml:

Line 60:         If you already have a <keyword keyref="hadoop_distro"/> environment set up and just need to add Impala to it, follow the installation
> This doesn't seem to work. It's showing up as "CDH" in the build.
Done


PS1, Line 698: from the sample TPC-DS kit for Impala.
> The only one of these is Cloudera's and it was very broken last I checked.
It may be that this section has to get removed due to bitrot on the TPC-DS stuff. If so, I'll suggest we do that in a subsequent CR and restrict ourselves to getting rid of explicit Cloudera / CDH references in this one.


PS1, Line 1700: (Currently, this technique only works for Parquet files.)
> Parentheses not necessary.
In this case, the parens are like a short form ot the "Note: this technique doesn't work for non-Parquet files" as we might do it in another section. I relax some of the rules a little bit (allowing royal 'we' etc.) in tutorial-style content.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I44245b65ce6f247ae8771f582f4b33c3712145ae
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Patch references to Cloudera and CDH in Impala tutorial

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.

Change subject: Patch references to Cloudera and CDH in Impala tutorial
......................................................................


Patch references to Cloudera and CDH in Impala tutorial

There was one tutorial that actually ran under the
'cloudera' user and so repeated that name over and
over in directory and HDFS paths. I switched that to
'username'.

I suppressed some <note> and <li> tags with Cloudera
Manager-specific details. Will physically remove those
from the source in a subsequent iteration.

I left several instances of audience="Cloudera" because
those will be changed to audience="hidden" as part of
a separate change request.

I marked with rev="upstream" some <codeblock> tags
containing impala-shell banners with a Cloudera copyright
statement. Will decide on a convention to handle those
(elide those lines, or use a conref to consistently
substitute the generic equivalent) and do that in a
followup patch set.

Change-Id: I44245b65ce6f247ae8771f582f4b33c3712145ae
Reviewed-on: http://gerrit.cloudera.org:8080/5663
Reviewed-by: Laurel Hale <la...@cloudera.com>
Reviewed-by: Jim Apple <jb...@apache.org>
Tested-by: Impala Public Jenkins
---
M docs/impala_keydefs.ditamap
M docs/topics/impala_tutorial.xml
2 files changed, 27 insertions(+), 27 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Jim Apple: Looks good to me, approved
  Laurel Hale: Looks good to me, but someone else must approve



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I44245b65ce6f247ae8771f582f4b33c3712145ae
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>

[Impala-ASF-CR] Patch references to Cloudera and CDH in Impala tutorial

Posted by "John Russell (Code Review)" <ge...@cloudera.org>.
Hello Laurel Hale,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/5663

to look at the new patch set (#2).

Change subject: Patch references to Cloudera and CDH in Impala tutorial
......................................................................

Patch references to Cloudera and CDH in Impala tutorial

There was one tutorial that actually ran under the
'cloudera' user and so repeated that name over and
over in directory and HDFS paths. I switched that to
'username'.

I suppressed some <note> and <li> tags with Cloudera
Manager-specific details. Will physically remove those
from the source in a subsequent iteration.

I left several instances of audience="Cloudera" because
those will be changed to audience="hidden" as part of
a separate change request.

I marked with rev="upstream" some <codeblock> tags
containing impala-shell banners with a Cloudera copyright
statement. Will decide on a convention to handle those
(elide those lines, or use a conref to consistently
substitute the generic equivalent) and do that in a
followup patch set.

Change-Id: I44245b65ce6f247ae8771f582f4b33c3712145ae
---
M docs/impala_keydefs.ditamap
M docs/topics/impala_tutorial.xml
2 files changed, 26 insertions(+), 26 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/5663/2
-- 
To view, visit http://gerrit.cloudera.org:8080/5663
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I44245b65ce6f247ae8771f582f4b33c3712145ae
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>

[Impala-ASF-CR] Patch references to Cloudera and CDH in Impala tutorial

Posted by "John Russell (Code Review)" <ge...@cloudera.org>.
John Russell has posted comments on this change.

Change subject: Patch references to Cloudera and CDH in Impala tutorial
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5663/2/docs/topics/impala_tutorial.xml
File docs/topics/impala_tutorial.xml:

PS2, Line 60: s
> This should be "an" since the value that replaces "hadoop_distro" begins wi
Done. I'll change it to "have some..." to avoid the dependency on "a" vs. "an".


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I44245b65ce6f247ae8771f582f4b33c3712145ae
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Patch references to Cloudera and CDH in Impala tutorial

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: Patch references to Cloudera and CDH in Impala tutorial
......................................................................


Patch Set 3:

Build started: http://jenkins.impala.io:8080/job/gerrit-docs-submit/46/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I44245b65ce6f247ae8771f582f4b33c3712145ae
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] Patch references to Cloudera and CDH in Impala tutorial

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: Patch references to Cloudera and CDH in Impala tutorial
......................................................................


Patch Set 3:

Laurel, I am satisfied if you are.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I44245b65ce6f247ae8771f582f4b33c3712145ae
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] Patch references to Cloudera and CDH in Impala tutorial

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: Patch references to Cloudera and CDH in Impala tutorial
......................................................................


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I44245b65ce6f247ae8771f582f4b33c3712145ae
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-HasComments: No