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 2014/01/13 17:42:02 UTC

Review Request 16821: SQOOP-1266 Sqoop2: Introduce top level commands

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

Review request for Sqoop.


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


Repository: sqoop-sqoop2


Description
-------

I've provided sqoop2-$commnand top level scripts for $command in "shell", "tool" and "server". I've chosen to encode the major version in the command name to make it easy to have side by side installation of both Sqoop 1 and 2 (e.g. sqoop2-tool instead of sqoop-tool). For the time being I've made the scripts very simple and just let them call the existing sqoop.sh. I would expect to remove the sqoop.sh in the future and move the entire functionality to these new top level scripts.

I've also updated the documentation to use the new commands.


Diffs
-----

  docs/src/site/sphinx/CommandLineClient.rst 22d6d4759ea5ec8624be08845693d47c0b607ea6 
  docs/src/site/sphinx/Installation.rst 97af41223f3528e32d3681b1e0ae9d618f2d8f37 
  docs/src/site/sphinx/Sqoop5MinutesDemo.rst 49e554b6613cbed740fe1ce336d9c60d1bba5d79 
  docs/src/site/sphinx/Tools.rst 1c7481fb895d0cd16686fd511a03786cef568630 
  docs/src/site/sphinx/Upgrade.rst 45d27bd8f1caccd0697c6b310b84a8f2ac74603d 

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


Testing
-------

Verified the new commands on real cluster.


Thanks,

Jarek Cecho

Re: Review Request 16821: SQOOP-1266 Sqoop2: Introduce top level commands

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


> On Jan. 13, 2014, 6:09 p.m., Abraham Elmahrek wrote:
> > dist/src/main/bin/sqoop2-server, lines 22-30
> > <https://reviews.apache.org/r/16821/diff/2/?file=421425#file421425line22>
> >
> >     Looks like you're resolving the symbolic link? This doesn't do any loop detection I think. Maybe use readlink -f (limited to linux flavors)?

Sadly the "readlink -f" is a linux only command that do not work on other platforms. We actually had this call in earlier versions, but had to remove it as it wasn't even working on Mac OS X. I believe that this particular while statement had been provided by the BigTop folks as a replacement for the missing "readlink -f".


> On Jan. 13, 2014, 6:09 p.m., Abraham Elmahrek wrote:
> > docs/src/site/sphinx/Installation.rst, line 45
> > <https://reviews.apache.org/r/16821/diff/2/?file=421429#file421429line45>
> >
> >     nix white space

Good catch, will fix.


- Jarek


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


On Jan. 13, 2014, 4:43 p.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16821/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2014, 4:43 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1266
>     https://issues.apache.org/jira/browse/SQOOP-1266
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> I've provided sqoop2-$commnand top level scripts for $command in "shell", "tool" and "server". I've chosen to encode the major version in the command name to make it easy to have side by side installation of both Sqoop 1 and 2 (e.g. sqoop2-tool instead of sqoop-tool). For the time being I've made the scripts very simple and just let them call the existing sqoop.sh. I would expect to remove the sqoop.sh in the future and move the entire functionality to these new top level scripts.
> 
> I've also updated the documentation to use the new commands.
> 
> 
> Diffs
> -----
> 
>   dist/src/main/bin/sqoop2-server PRE-CREATION 
>   dist/src/main/bin/sqoop2-shell PRE-CREATION 
>   dist/src/main/bin/sqoop2-tool PRE-CREATION 
>   docs/src/site/sphinx/CommandLineClient.rst 22d6d4759ea5ec8624be08845693d47c0b607ea6 
>   docs/src/site/sphinx/Installation.rst 97af41223f3528e32d3681b1e0ae9d618f2d8f37 
>   docs/src/site/sphinx/Sqoop5MinutesDemo.rst 49e554b6613cbed740fe1ce336d9c60d1bba5d79 
>   docs/src/site/sphinx/Tools.rst 1c7481fb895d0cd16686fd511a03786cef568630 
>   docs/src/site/sphinx/Upgrade.rst 45d27bd8f1caccd0697c6b310b84a8f2ac74603d 
> 
> Diff: https://reviews.apache.org/r/16821/diff/
> 
> 
> Testing
> -------
> 
> Verified the new commands on real cluster.
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Re: Review Request 16821: SQOOP-1266 Sqoop2: Introduce top level commands

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

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

Ship it!


Good stuff man. Just a thought on symlink resolution... Not sure which is the preferred method though.


dist/src/main/bin/sqoop2-server
<https://reviews.apache.org/r/16821/#comment60269>

    Looks like you're resolving the symbolic link? This doesn't do any loop detection I think. Maybe use readlink -f (limited to linux flavors)?



dist/src/main/bin/sqoop2-shell
<https://reviews.apache.org/r/16821/#comment60271>

    Same as above



dist/src/main/bin/sqoop2-tool
<https://reviews.apache.org/r/16821/#comment60272>

    Same as above



docs/src/site/sphinx/Installation.rst
<https://reviews.apache.org/r/16821/#comment60273>

    nix white space


- Abraham Elmahrek


On Jan. 13, 2014, 4:43 p.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16821/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2014, 4:43 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1266
>     https://issues.apache.org/jira/browse/SQOOP-1266
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> I've provided sqoop2-$commnand top level scripts for $command in "shell", "tool" and "server". I've chosen to encode the major version in the command name to make it easy to have side by side installation of both Sqoop 1 and 2 (e.g. sqoop2-tool instead of sqoop-tool). For the time being I've made the scripts very simple and just let them call the existing sqoop.sh. I would expect to remove the sqoop.sh in the future and move the entire functionality to these new top level scripts.
> 
> I've also updated the documentation to use the new commands.
> 
> 
> Diffs
> -----
> 
>   dist/src/main/bin/sqoop2-server PRE-CREATION 
>   dist/src/main/bin/sqoop2-shell PRE-CREATION 
>   dist/src/main/bin/sqoop2-tool PRE-CREATION 
>   docs/src/site/sphinx/CommandLineClient.rst 22d6d4759ea5ec8624be08845693d47c0b607ea6 
>   docs/src/site/sphinx/Installation.rst 97af41223f3528e32d3681b1e0ae9d618f2d8f37 
>   docs/src/site/sphinx/Sqoop5MinutesDemo.rst 49e554b6613cbed740fe1ce336d9c60d1bba5d79 
>   docs/src/site/sphinx/Tools.rst 1c7481fb895d0cd16686fd511a03786cef568630 
>   docs/src/site/sphinx/Upgrade.rst 45d27bd8f1caccd0697c6b310b84a8f2ac74603d 
> 
> Diff: https://reviews.apache.org/r/16821/diff/
> 
> 
> Testing
> -------
> 
> Verified the new commands on real cluster.
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Re: Review Request 16821: SQOOP-1266 Sqoop2: Introduce top level commands

Posted by Hari Shreedharan <hs...@cloudera.com>.

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

Ship it!


Ship It!

- Hari Shreedharan


On Jan. 14, 2014, 5:45 a.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16821/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2014, 5:45 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1266
>     https://issues.apache.org/jira/browse/SQOOP-1266
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> I've provided sqoop2-$commnand top level scripts for $command in "shell", "tool" and "server". I've chosen to encode the major version in the command name to make it easy to have side by side installation of both Sqoop 1 and 2 (e.g. sqoop2-tool instead of sqoop-tool). For the time being I've made the scripts very simple and just let them call the existing sqoop.sh. I would expect to remove the sqoop.sh in the future and move the entire functionality to these new top level scripts.
> 
> I've also updated the documentation to use the new commands.
> 
> 
> Diffs
> -----
> 
>   dist/src/main/bin/sqoop2-server PRE-CREATION 
>   dist/src/main/bin/sqoop2-shell PRE-CREATION 
>   dist/src/main/bin/sqoop2-tool PRE-CREATION 
>   docs/src/site/sphinx/CommandLineClient.rst 22d6d4759ea5ec8624be08845693d47c0b607ea6 
>   docs/src/site/sphinx/Installation.rst 97af41223f3528e32d3681b1e0ae9d618f2d8f37 
>   docs/src/site/sphinx/Sqoop5MinutesDemo.rst 49e554b6613cbed740fe1ce336d9c60d1bba5d79 
>   docs/src/site/sphinx/Tools.rst 1c7481fb895d0cd16686fd511a03786cef568630 
>   docs/src/site/sphinx/Upgrade.rst 45d27bd8f1caccd0697c6b310b84a8f2ac74603d 
> 
> Diff: https://reviews.apache.org/r/16821/diff/
> 
> 
> Testing
> -------
> 
> Verified the new commands on real cluster.
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Re: Review Request 16821: SQOOP-1266 Sqoop2: Introduce top level commands

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


> On Jan. 16, 2014, 7:23 p.m., Hari Shreedharan wrote:
> > For all 3 scripts, most of the script is copy-paste. Is there a reason we can't put it in one script and simply source that script?

Good point Hari! I was trying to do exactly that, however there is a chicken-egg problem. In order to reliably source a script from the same directory I need that fragment in order to get the "current" directory. Hence the copy-paste into all three scripts.


- Jarek


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


On Jan. 14, 2014, 5:45 a.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16821/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2014, 5:45 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1266
>     https://issues.apache.org/jira/browse/SQOOP-1266
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> I've provided sqoop2-$commnand top level scripts for $command in "shell", "tool" and "server". I've chosen to encode the major version in the command name to make it easy to have side by side installation of both Sqoop 1 and 2 (e.g. sqoop2-tool instead of sqoop-tool). For the time being I've made the scripts very simple and just let them call the existing sqoop.sh. I would expect to remove the sqoop.sh in the future and move the entire functionality to these new top level scripts.
> 
> I've also updated the documentation to use the new commands.
> 
> 
> Diffs
> -----
> 
>   dist/src/main/bin/sqoop2-server PRE-CREATION 
>   dist/src/main/bin/sqoop2-shell PRE-CREATION 
>   dist/src/main/bin/sqoop2-tool PRE-CREATION 
>   docs/src/site/sphinx/CommandLineClient.rst 22d6d4759ea5ec8624be08845693d47c0b607ea6 
>   docs/src/site/sphinx/Installation.rst 97af41223f3528e32d3681b1e0ae9d618f2d8f37 
>   docs/src/site/sphinx/Sqoop5MinutesDemo.rst 49e554b6613cbed740fe1ce336d9c60d1bba5d79 
>   docs/src/site/sphinx/Tools.rst 1c7481fb895d0cd16686fd511a03786cef568630 
>   docs/src/site/sphinx/Upgrade.rst 45d27bd8f1caccd0697c6b310b84a8f2ac74603d 
> 
> Diff: https://reviews.apache.org/r/16821/diff/
> 
> 
> Testing
> -------
> 
> Verified the new commands on real cluster.
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Re: Review Request 16821: SQOOP-1266 Sqoop2: Introduce top level commands

Posted by Hari Shreedharan <hs...@cloudera.com>.

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


For all 3 scripts, most of the script is copy-paste. Is there a reason we can't put it in one script and simply source that script?

- Hari Shreedharan


On Jan. 14, 2014, 5:45 a.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16821/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2014, 5:45 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1266
>     https://issues.apache.org/jira/browse/SQOOP-1266
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> I've provided sqoop2-$commnand top level scripts for $command in "shell", "tool" and "server". I've chosen to encode the major version in the command name to make it easy to have side by side installation of both Sqoop 1 and 2 (e.g. sqoop2-tool instead of sqoop-tool). For the time being I've made the scripts very simple and just let them call the existing sqoop.sh. I would expect to remove the sqoop.sh in the future and move the entire functionality to these new top level scripts.
> 
> I've also updated the documentation to use the new commands.
> 
> 
> Diffs
> -----
> 
>   dist/src/main/bin/sqoop2-server PRE-CREATION 
>   dist/src/main/bin/sqoop2-shell PRE-CREATION 
>   dist/src/main/bin/sqoop2-tool PRE-CREATION 
>   docs/src/site/sphinx/CommandLineClient.rst 22d6d4759ea5ec8624be08845693d47c0b607ea6 
>   docs/src/site/sphinx/Installation.rst 97af41223f3528e32d3681b1e0ae9d618f2d8f37 
>   docs/src/site/sphinx/Sqoop5MinutesDemo.rst 49e554b6613cbed740fe1ce336d9c60d1bba5d79 
>   docs/src/site/sphinx/Tools.rst 1c7481fb895d0cd16686fd511a03786cef568630 
>   docs/src/site/sphinx/Upgrade.rst 45d27bd8f1caccd0697c6b310b84a8f2ac74603d 
> 
> Diff: https://reviews.apache.org/r/16821/diff/
> 
> 
> Testing
> -------
> 
> Verified the new commands on real cluster.
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Re: Review Request 16821: SQOOP-1266 Sqoop2: Introduce top level commands

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

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

(Updated Jan. 14, 2014, 5:45 a.m.)

Review request for Sqoop.

Changes
-------

Removed trailing white space.

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

Repository: sqoop-sqoop2

Description
-------

I've provided sqoop2-$commnand top level scripts for $command in "shell", "tool" and "server". I've chosen to encode the major version in the command name to make it easy to have side by side installation of both Sqoop 1 and 2 (e.g. sqoop2-tool instead of sqoop-tool). For the time being I've made the scripts very simple and just let them call the existing sqoop.sh. I would expect to remove the sqoop.sh in the future and move the entire functionality to these new top level scripts.

I've also updated the documentation to use the new commands.

Diffs (updated)
-----

dist/src/main/bin/sqoop2-server PRE-CREATION
dist/src/main/bin/sqoop2-shell PRE-CREATION
dist/src/main/bin/sqoop2-tool PRE-CREATION
docs/src/site/sphinx/CommandLineClient.rst 22d6d4759ea5ec8624be08845693d47c0b607ea6
docs/src/site/sphinx/Installation.rst 97af41223f3528e32d3681b1e0ae9d618f2d8f37
docs/src/site/sphinx/Sqoop5MinutesDemo.rst 49e554b6613cbed740fe1ce336d9c60d1bba5d79
docs/src/site/sphinx/Tools.rst 1c7481fb895d0cd16686fd511a03786cef568630
docs/src/site/sphinx/Upgrade.rst 45d27bd8f1caccd0697c6b310b84a8f2ac74603d

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

Testing
-------

Verified the new commands on real cluster.

Thanks,

Jarek Cecho

Re: Review Request 16821: SQOOP-1266 Sqoop2: Introduce top level commands

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

(Updated Jan. 13, 2014, 4:43 p.m.)

Review request for Sqoop.

Changes
-------

Adding missing new files.

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

Repository: sqoop-sqoop2

Description
-------

I've also updated the documentation to use the new commands.

Diffs (updated)
-----

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

Testing
-------

Verified the new commands on real cluster.

Thanks,

Jarek Cecho