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 2013/06/02 16:23:49 UTC

Re: Review Request: SQOOP-973 add --url option for 'set server', which enables specifying host, port and webapp in a more consistant way with other applications. For example, 'set server --url http://localhost:12000/sqoop'


> On May 30, 2013, 5:23 a.m., Abraham Elmahrek wrote:
> > client/src/main/java/org/apache/sqoop/client/shell/ShellEnvironment.java, line 111
> > <https://reviews.apache.org/r/11535/diff/1/?file=298453#file298453line111>
> >
> >     Is there a default port number constant? If not, I think it should be so that it can be referenced by other methods.
> 
> Mengwei Ding wrote:
>     At the very first beginning of this file, the default port and webapp are also set in this way. I checked the references of default port, too. Seems like it's been referenced in no other place.
>

Do you think that it would make sense to define a variable with default value and then just reference it instead of using the getEnv each time? Something like:

private static String DEFAULT_SERVER_HOST = getEnv(Constants.ENV_HOST, "localhost");

...

private static String serverHost = DEFAULT_SERVER_HOST;
...

public static void setServerUrl(String ustr) {
...
  serverPort = DEFAULT_SERVER_HOST;
...


- Jarek


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


On May 30, 2013, 3:14 a.m., Mengwei Ding wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11535/
> -----------------------------------------------------------
> 
> (Updated May 30, 2013, 3:14 a.m.)
> 
> 
> Review request for Sqoop, Jarek Cecho, Hari Shreedharan, and Abraham Elmahrek.
> 
> 
> Description
> -------
> 
> commit 42577e8b5e2de3c987dfd99d462a4e766f617729
> Author: Mengwei Ding <me...@cloudera.com>
> Date:   Wed May 29 18:04:53 2013 -0700
> 
>     SQOOP-973 add --url option for 'set server', which enables specifying host, port and webapp in a more consistant way with other applications. For example, 'set server --url http://localhost:12000/sqoop'
> 
> :100644 100644 8c3c6a4... e0956a0... M	client/src/main/java/org/apache/sqoop/client/core/Constants.java
> :100644 100644 1c85592... c2581c3... M	client/src/main/java/org/apache/sqoop/client/shell/SetServerFunction.java
> :100644 100644 f6d1267... 44007d1... M	client/src/main/java/org/apache/sqoop/client/shell/ShellEnvironment.java
> :100644 100644 c7a4cb3... 2a5eea4... M	client/src/main/resources/client-resource.properties
> 
> 
> This addresses bug SQOOP-973.
>     https://issues.apache.org/jira/browse/SQOOP-973
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/sqoop/client/core/Constants.java 8c3c6a4 
>   client/src/main/java/org/apache/sqoop/client/shell/SetServerFunction.java 1c85592 
>   client/src/main/java/org/apache/sqoop/client/shell/ShellEnvironment.java f6d1267 
>   client/src/main/resources/client-resource.properties c7a4cb3 
> 
> Diff: https://reviews.apache.org/r/11535/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Mengwei Ding
> 
>