> On March 30, 2013, 4:46 a.m., Abraham Elmahrek wrote:
> > This looks good to me with the exception of one thought. The client will be 
> > connecting to some RESTful resource, or at least Web like resource, ( IE: 
> > http://%(host)s:%(port)d/%(resource) ). Does it make sense to use idioms 
> > that reference a URI naming conventions?
> > 
> > +1

Unfortunately current shell is strictly separating host, port and resource, so 
I wanted to stay consistent. But I'm totally open to change that as it seems 
kind of weird. I'll create follow up jira for that.


> On March 30, 2013, 4:46 a.m., Abraham Elmahrek wrote:
> > client/src/main/java/org/apache/sqoop/client/core/Constants.java, line 33
> > <https://reviews.apache.org/r/10203/diff/1/?file=276491#file276491line33>
> >
> >     SQOOP2_RESOURCE?

Yeah totally agree, the "SQOOP2_APP" is confusing. I've changed it to 
"SQOOP2_WEBAPP" so that we're consistent with the rest of the shell, but I 
don't like it either. Maybe we can create follow up jira to change the "webapp" 
to "resource" across entire shell?


- Jarek


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


On March 30, 2013, 8:55 p.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10203/
> -----------------------------------------------------------
> 
> (Updated March 30, 2013, 8:55 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Description
> -------
> 
> I've created environmental variables that will be fetched for Sqoop2 server 
> default hostname (port, ...). Please note that resource file (if it exists) 
> will have precedence over the environmental variables in current 
> implementation.
> 
> 
> This addresses bug SQOOP-972.
>     https://issues.apache.org/jira/browse/SQOOP-972
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/sqoop/client/core/Constants.java 
> cc956d0f9306a08ef9084e7b878f82e3e355d617 
>   client/src/main/java/org/apache/sqoop/client/shell/ShellEnvironment.java 
> 25ae364641c7a03e8394ac14991370fc6f810745 
> 
> Diff: https://reviews.apache.org/r/10203/diff/
> 
> 
> Testing
> -------
> 
> I've done manual testing as we currently do not have tests for shell 
> component.
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Reply via email to