You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Jarek Cecho <ja...@apache.org> on 2015/11/21 16:11:33 UTC

Review Request 40576: SQOOP-2708 Sqoop2: Docs: Update Installation.rst

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40576/
-----------------------------------------------------------

Review request for Sqoop.


Bugs: SQOOP-2708
    https://issues.apache.org/jira/browse/SQOOP-2708


Repository: sqoop-sqoop2


Description
-------

I've updated the installation guide and incoporated changes that were missing.


Diffs
-----

  docs/src/site/sphinx/admin/Installation.rst 9d56875 
  docs/src/site/sphinx/admin/Tools.rst fb0187a 

Diff: https://reviews.apache.org/r/40576/diff/


Testing
-------


Thanks,

Jarek Cecho


Re: Review Request 40576: SQOOP-2708 Sqoop2: Docs: Update Installation.rst

Posted by Jarek Cecho <ja...@apache.org>.

> On Nov. 30, 2015, 6:50 p.m., Abraham Fine wrote:
> > docs/src/site/sphinx/admin/Installation.rst, line 69
> > <https://reviews.apache.org/r/40576/diff/2/?file=1147100#file1147100line69>
> >
> >     "Sqoop server requires further configuration on Hadoop side" is not needed.
> >     
> >     I would split this section into two subsections: "HDFS Proxy Users" and "YARN Whitelist".

I've nuked the introduction sentence - I already do have two logical sections for Proxy/Whitelist already. I didn't want to introduce yet another nested structure though so I have it under one "headline".


> On Nov. 30, 2015, 6:50 p.m., Abraham Fine wrote:
> > docs/src/site/sphinx/admin/Installation.rst, line 71
> > <https://reviews.apache.org/r/40576/diff/2/?file=1147100#file1147100line71>
> >
> >     does this always need to be enabled. could there be use cases (such as moving data for s3 to an rdbms) where the cluster does not need to be configured to allow impersonation?

The reality is that only if you're using HDFS connector this will be needed. But as that is so common use case, I did not wanted to burry that inside the HDFS connector docs but rather expose it here in the installation docs.


> On Nov. 30, 2015, 6:50 p.m., Abraham Fine wrote:
> > docs/src/site/sphinx/admin/Installation.rst, line 97
> > <https://reviews.apache.org/r/40576/diff/2/?file=1147100#file1147100line97>
> >
> >     I would describe the use case here in a little bit more detail. We have a few different ways of loading jars into sqoop 2. Perhaps we should move this out of the installation documentation into its own page to keep initial installation simple.

True, we also have ability to configure jars in jobs and such. Would you mind if I'll keep it this way for now and add a standalone section about classpath later? (especially after Dian will be done with connector classpath isolation project).


> On Nov. 30, 2015, 6:50 p.m., Abraham Fine wrote:
> > docs/src/site/sphinx/admin/Tools.rst, line 60
> > <https://reviews.apache.org/r/40576/diff/2/?file=1147101#file1147101line60>
> >
> >     "the same file"

Let's fix the Tools documentation separately - I'm editing the file only to add anchors at this point.


> On Nov. 30, 2015, 6:50 p.m., Abraham Fine wrote:
> > docs/src/site/sphinx/admin/Tools.rst, line 67
> > <https://reviews.apache.org/r/40576/diff/2/?file=1147101#file1147101line67>
> >
> >     Perhaps we should make it clear that we are not actually downloading and installing a new version of sqoop 2 as a side node?
> >     
> >     Really this is a "repository upgrade tool".

Let's fix the Tools documentation separately - I'm editing the file only to add anchors at this point.


- Jarek


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40576/#review108352
-----------------------------------------------------------


On Nov. 25, 2015, 7:40 p.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40576/
> -----------------------------------------------------------
> 
> (Updated Nov. 25, 2015, 7:40 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2708
>     https://issues.apache.org/jira/browse/SQOOP-2708
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> I've updated the installation guide and incoporated changes that were missing.
> 
> 
> Diffs
> -----
> 
>   docs/src/site/sphinx/admin/Installation.rst 9d56875 
>   docs/src/site/sphinx/admin/Tools.rst fb0187a 
> 
> Diff: https://reviews.apache.org/r/40576/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>


Re: Review Request 40576: SQOOP-2708 Sqoop2: Docs: Update Installation.rst

Posted by Abraham Fine <ab...@cloudera.com>.

> On Nov. 30, 2015, 6:50 p.m., Abraham Fine wrote:
> > docs/src/site/sphinx/admin/Installation.rst, line 97
> > <https://reviews.apache.org/r/40576/diff/2/?file=1147100#file1147100line97>
> >
> >     I would describe the use case here in a little bit more detail. We have a few different ways of loading jars into sqoop 2. Perhaps we should move this out of the installation documentation into its own page to keep initial installation simple.
> 
> Jarek Cecho wrote:
>     True, we also have ability to configure jars in jobs and such. Would you mind if I'll keep it this way for now and add a standalone section about classpath later? (especially after Dian will be done with connector classpath isolation project).

ok, let's make a jira for that so we do not forget about it.


- Abraham


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40576/#review108352
-----------------------------------------------------------


On Nov. 30, 2015, 8:31 p.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40576/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2015, 8:31 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2708
>     https://issues.apache.org/jira/browse/SQOOP-2708
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> I've updated the installation guide and incoporated changes that were missing.
> 
> 
> Diffs
> -----
> 
>   docs/src/site/sphinx/admin/Installation.rst 9d56875 
>   docs/src/site/sphinx/admin/Tools.rst fb0187a 
> 
> Diff: https://reviews.apache.org/r/40576/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>


Re: Review Request 40576: SQOOP-2708 Sqoop2: Docs: Update Installation.rst

Posted by Jarek Cecho <ja...@apache.org>.

> On Nov. 30, 2015, 6:50 p.m., Abraham Fine wrote:
> > docs/src/site/sphinx/admin/Installation.rst, line 97
> > <https://reviews.apache.org/r/40576/diff/2/?file=1147100#file1147100line97>
> >
> >     I would describe the use case here in a little bit more detail. We have a few different ways of loading jars into sqoop 2. Perhaps we should move this out of the installation documentation into its own page to keep initial installation simple.
> 
> Jarek Cecho wrote:
>     True, we also have ability to configure jars in jobs and such. Would you mind if I'll keep it this way for now and add a standalone section about classpath later? (especially after Dian will be done with connector classpath isolation project).
> 
> Abraham Fine wrote:
>     ok, let's make a jira for that so we do not forget about it.

Agreed: https://issues.apache.org/jira/browse/SQOOP-2716


- Jarek


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40576/#review108352
-----------------------------------------------------------


On Nov. 30, 2015, 8:31 p.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40576/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2015, 8:31 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2708
>     https://issues.apache.org/jira/browse/SQOOP-2708
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> I've updated the installation guide and incoporated changes that were missing.
> 
> 
> Diffs
> -----
> 
>   docs/src/site/sphinx/admin/Installation.rst 9d56875 
>   docs/src/site/sphinx/admin/Tools.rst fb0187a 
> 
> Diff: https://reviews.apache.org/r/40576/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>


Re: Review Request 40576: SQOOP-2708 Sqoop2: Docs: Update Installation.rst

Posted by Abraham Fine <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40576/#review108352
-----------------------------------------------------------



docs/src/site/sphinx/admin/Installation.rst (line 24)
<https://reviews.apache.org/r/40576/#comment167774>

    "any number of machines". 
    
    no need for 'arbitrary'



docs/src/site/sphinx/admin/Installation.rst (line 29)
<https://reviews.apache.org/r/40576/#comment167775>

    Copy the Sqoop artifact to the machine where you want to run Sqoop server. The Sqoop server acts as a Hadoop client, therefore Hadoop libraries (Yarn, Mapreduce, and HDFS jar files) and configuration files (...) must be available on this node. You do not need to run any Hadoop related services - running the server on a "gateway" node is perfectly fine.



docs/src/site/sphinx/admin/Installation.rst (line 35)
<https://reviews.apache.org/r/40576/#comment167776>

    "or later"
    
    To install the Sqoop server, decompress the tarball (in a location of your choosing) and set the newly created forder as your working directory



docs/src/site/sphinx/admin/Installation.rst (line 52)
<https://reviews.apache.org/r/40576/#comment167780>

    In this chunk of documentation we seem to describe a more complicated configuration pattern before describing the simpler one. I would reverse this.
    
    The Sqoop server uses environment variables to find Hadoop libraries. If the environment variable ``$HADOOP_HOME`` is set, Sqoop will look for jars in the follwing locations: ``$HADOOP_HOME/share/hadoop/common``, ``$HADOOP_HOME/share/hadoop/hdfs``, ``$HADOOP_HOME/share/hadoop/mapreduce`` and ``$HADOOP_HOME/share/hadoop/yarn``. You can specify where the Sqoop server should look for the common, hdfs, mapreduce, and yarn jars indepently with the ``$HADOOP_COMMON_HOME``, ``$HADOOP_HDFS_HOME``, ``$HADOOP_MAPRED_HOME`` and ``$HADOOP_YARN_HOME`` environment variables.



docs/src/site/sphinx/admin/Installation.rst (line 69)
<https://reviews.apache.org/r/40576/#comment167782>

    "Sqoop server requires further configuration on Hadoop side" is not needed.
    
    I would split this section into two subsections: "HDFS Proxy Users" and "YARN Whitelist".



docs/src/site/sphinx/admin/Installation.rst (line 71)
<https://reviews.apache.org/r/40576/#comment167783>

    does this always need to be enabled. could there be use cases (such as moving data for s3 to an rdbms) where the cluster does not need to be configured to allow impersonation?



docs/src/site/sphinx/admin/Installation.rst (line 97)
<https://reviews.apache.org/r/40576/#comment167785>

    I would describe the use case here in a little bit more detail. We have a few different ways of loading jars into sqoop 2. Perhaps we should move this out of the installation documentation into its own page to keep initial installation simple.



docs/src/site/sphinx/admin/Installation.rst (line 118)
<https://reviews.apache.org/r/40576/#comment167786>

    "for easier execution"



docs/src/site/sphinx/admin/Installation.rst (line 124)
<https://reviews.apache.org/r/40576/#comment167787>

    lets be consistent for shell commands vs binaries.
    
    "The remainder of the Sqoop 2 documentation assumes that the shell commands are in your ``$PATH``."
    
    I would remove the last sentence, I think the point has been already made clear.



docs/src/site/sphinx/admin/Installation.rst (line 129)
<https://reviews.apache.org/r/40576/#comment167788>

    I don't think the first sentence is necessary. I think that is made clear by our documentation showing the start step after this step.



docs/src/site/sphinx/admin/Installation.rst (line 135)
<https://reviews.apache.org/r/40576/#comment167789>

    Rename this section "Repository Initialization"



docs/src/site/sphinx/admin/Installation.rst (line 138)
<https://reviews.apache.org/r/40576/#comment167793>

    "The metadata repository needs to be initialized. The tool-upgrade can be used..."



docs/src/site/sphinx/admin/Installation.rst (line 144)
<https://reviews.apache.org/r/40576/#comment167794>

    "for first execution" is not necessary



docs/src/site/sphinx/admin/Installation.rst (line 156)
<https://reviews.apache.org/r/40576/#comment167795>

    "the follwing command"



docs/src/site/sphinx/admin/Installation.rst (line 162)
<https://reviews.apache.org/r/40576/#comment167796>

    Remove "similarly" add "the"



docs/src/site/sphinx/admin/Installation.rst (line 168)
<https://reviews.apache.org/r/40576/#comment167797>

    "daemons" and "ports" do not need to be plural.



docs/src/site/sphinx/admin/Installation.rst (line 173)
<https://reviews.apache.org/r/40576/#comment167799>

    Put the first sentence as a note at the bottom of the section. 
    
    I think the documentation is clearer if we first state what we will be accomplishing and then resolving common misconceptions after.



docs/src/site/sphinx/admin/Installation.rst (line 179)
<https://reviews.apache.org/r/40576/#comment167800>

    "documentation for Sqoop shell"



docs/src/site/sphinx/admin/Tools.rst (line 60)
<https://reviews.apache.org/r/40576/#comment167801>

    "the same file"



docs/src/site/sphinx/admin/Tools.rst (line 67)
<https://reviews.apache.org/r/40576/#comment167802>

    Perhaps we should make it clear that we are not actually downloading and installing a new version of sqoop 2 as a side node?
    
    Really this is a "repository upgrade tool".


- Abraham Fine


On Nov. 25, 2015, 7:40 p.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40576/
> -----------------------------------------------------------
> 
> (Updated Nov. 25, 2015, 7:40 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2708
>     https://issues.apache.org/jira/browse/SQOOP-2708
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> I've updated the installation guide and incoporated changes that were missing.
> 
> 
> Diffs
> -----
> 
>   docs/src/site/sphinx/admin/Installation.rst 9d56875 
>   docs/src/site/sphinx/admin/Tools.rst fb0187a 
> 
> Diff: https://reviews.apache.org/r/40576/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>


Re: Review Request 40576: SQOOP-2708 Sqoop2: Docs: Update Installation.rst

Posted by Abraham Fine <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40576/#review108399
-----------------------------------------------------------

Ship it!


Ship It!

- Abraham Fine


On Nov. 30, 2015, 8:31 p.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40576/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2015, 8:31 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2708
>     https://issues.apache.org/jira/browse/SQOOP-2708
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> I've updated the installation guide and incoporated changes that were missing.
> 
> 
> Diffs
> -----
> 
>   docs/src/site/sphinx/admin/Installation.rst 9d56875 
>   docs/src/site/sphinx/admin/Tools.rst fb0187a 
> 
> Diff: https://reviews.apache.org/r/40576/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>


Re: Review Request 40576: SQOOP-2708 Sqoop2: Docs: Update Installation.rst

Posted by Kathleen Ting <ka...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40576/#review110567
-----------------------------------------------------------


Per the Sqoop QA bot, there's added whitespace (which wasn't there in prior versions of the patch). Best to remove before committing?

- Kathleen Ting


On Dec. 15, 2015, 7:42 a.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40576/
> -----------------------------------------------------------
> 
> (Updated Dec. 15, 2015, 7:42 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2708
>     https://issues.apache.org/jira/browse/SQOOP-2708
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> I've updated the installation guide and incoporated changes that were missing.
> 
> 
> Diffs
> -----
> 
>   docs/src/site/sphinx/admin/Installation.rst 383c4ec 
>   docs/src/site/sphinx/admin/Tools.rst 3b1f5ac 
> 
> Diff: https://reviews.apache.org/r/40576/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>


Re: Review Request 40576: SQOOP-2708 Sqoop2: Docs: Update Installation.rst

Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40576/
-----------------------------------------------------------

(Updated Dec. 15, 2015, 7:42 a.m.)


Review request for Sqoop.


Changes
-------

Rebased version on current HEAD


Bugs: SQOOP-2708
    https://issues.apache.org/jira/browse/SQOOP-2708


Repository: sqoop-sqoop2


Description
-------

I've updated the installation guide and incoporated changes that were missing.


Diffs (updated)
-----

  docs/src/site/sphinx/admin/Installation.rst 383c4ec 
  docs/src/site/sphinx/admin/Tools.rst 3b1f5ac 

Diff: https://reviews.apache.org/r/40576/diff/


Testing
-------


Thanks,

Jarek Cecho


Re: Review Request 40576: SQOOP-2708 Sqoop2: Docs: Update Installation.rst

Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40576/
-----------------------------------------------------------

(Updated Nov. 30, 2015, 8:31 p.m.)


Review request for Sqoop.


Changes
-------

Incorporated Abe's feedback.


Bugs: SQOOP-2708
    https://issues.apache.org/jira/browse/SQOOP-2708


Repository: sqoop-sqoop2


Description
-------

I've updated the installation guide and incoporated changes that were missing.


Diffs (updated)
-----

  docs/src/site/sphinx/admin/Installation.rst 9d56875 
  docs/src/site/sphinx/admin/Tools.rst fb0187a 

Diff: https://reviews.apache.org/r/40576/diff/


Testing
-------


Thanks,

Jarek Cecho


Re: Review Request 40576: SQOOP-2708 Sqoop2: Docs: Update Installation.rst

Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40576/
-----------------------------------------------------------

(Updated Nov. 25, 2015, 7:40 p.m.)


Review request for Sqoop.


Changes
-------

I've added passage talking about YARN user whitelist.


Bugs: SQOOP-2708
    https://issues.apache.org/jira/browse/SQOOP-2708


Repository: sqoop-sqoop2


Description
-------

I've updated the installation guide and incoporated changes that were missing.


Diffs (updated)
-----

  docs/src/site/sphinx/admin/Installation.rst 9d56875 
  docs/src/site/sphinx/admin/Tools.rst fb0187a 

Diff: https://reviews.apache.org/r/40576/diff/


Testing
-------


Thanks,

Jarek Cecho