> On March 25, 2016, 4:22 a.m., Sumit Mohanty wrote:
> > ambari-common/src/main/python/resource_management/libraries/functions/copy_tarball.py,
> >  line 50
> > <https://reviews.apache.org/r/45325/diff/1/?file=1314428#file1314428line50>
> >
> >     Are these paths verified with the Hive team?

Yes. Different folder. Same tar file name.


> On March 25, 2016, 4:22 a.m., Sumit Mohanty wrote:
> > ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/hive_interactive.py,
> >  line 57
> > <https://reviews.apache.org/r/45325/diff/1/?file=1314429#file1314429line57>
> >
> >     Is it copied for each start() command?

Yes. Happens same for Hive Batch also. I see a point in this if custom tar file 
is kept. It will be picked and copied. Your inputs ? I can fix it for both if 
required.


> On March 25, 2016, 4:22 a.m., Sumit Mohanty wrote:
> > ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/hive_interactive.py,
> >  line 94
> > <https://reviews.apache.org/r/45325/diff/1/?file=1314429#file1314429line94>
> >
> >     Nit: the formatting is off. Extra space for all lines after first one.

Fixed.


> On March 25, 2016, 4:22 a.m., Sumit Mohanty wrote:
> > ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/hive_interactive.py,
> >  line 108
> > <https://reviews.apache.org/r/45325/diff/1/?file=1314429#file1314429line108>
> >
> >     We should rename this variable - and also the variable above it 
> > "target". Rename them to be something more descriptive.

Named target_hive_interactive


> On March 25, 2016, 4:22 a.m., Sumit Mohanty wrote:
> > ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/hive_interactive.py,
> >  line 120
> > <https://reviews.apache.org/r/45325/diff/1/?file=1314429#file1314429line120>
> >
> >     Can we rename this also to create_directory()?

Fixed.


> On March 25, 2016, 4:22 a.m., Sumit Mohanty wrote:
> > ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/hive_server_interactive.py,
> >  line 97
> > <https://reviews.apache.org/r/45325/diff/1/?file=1314430#file1314430line97>
> >
> >     Use Execute() and pass in params as needed.

Fixed.


> On March 25, 2016, 4:22 a.m., Sumit Mohanty wrote:
> > ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/hive_server_interactive.py,
> >  line 112
> > <https://reviews.apache.org/r/45325/diff/1/?file=1314430#file1314430line112>
> >
> >     Lets remove the sleep - why does hive interactive need to have LLAP 
> > running? Even if we need a sleep it should be 10-30 seconds only

LLAP needs to be up and running (we may find LLAP in accepted state) if we 
don't wait based on the cluster load. Ultimately, we will check LLAP slider 
status status. There is a TODO/comment for that.


> On March 25, 2016, 4:22 a.m., Sumit Mohanty wrote:
> > ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/hive_server_interactive.py,
> >  line 115
> > <https://reviews.apache.org/r/45325/diff/1/?file=1314430#file1314430line115>
> >
> >     We should log the package name as well.

Fixed.


> On March 25, 2016, 4:22 a.m., Sumit Mohanty wrote:
> > ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/hive_server_interactive.py,
> >  line 118
> > <https://reviews.apache.org/r/45325/diff/1/?file=1314430#file1314430line118>
> >
> >     If we check for the file, should we check for the message? is there a 
> > way to control the output location of the file or give the file a name. It 
> > seems that this process is error prone. I would rather have full control on 
> > the file name or path and then check the existence of the file rather than 
> > message.

we can put a requirement on Slider/LLAP. Can't figure out a way as of now. 
Looks trivial if they can accept the supplied folder name


> On March 25, 2016, 4:22 a.m., Sumit Mohanty wrote:
> > ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/hive_server_interactive.py,
> >  line 155
> > <https://reviews.apache.org/r/45325/diff/1/?file=1314430#file1314430line155>
> >
> >     Use Execute()

Done.


> On March 25, 2016, 4:22 a.m., Sumit Mohanty wrote:
> > ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/hive_server_interactive.py,
> >  line 160
> > <https://reviews.apache.org/r/45325/diff/1/?file=1314430#file1314430line160>
> >
> >     Use Execute()

Done.


> On March 25, 2016, 4:22 a.m., Sumit Mohanty wrote:
> > ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/hive_server_interactive.py,
> >  line 80
> > <https://reviews.apache.org/r/45325/diff/1/?file=1314430#file1314430line80>
> >
> >     What's FOR SECURITY?

Nothing. Wrong comment. Removed.


> On March 25, 2016, 4:22 a.m., Sumit Mohanty wrote:
> > ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/hive_interactive.py,
> >  line 76
> > <https://reviews.apache.org/r/45325/diff/1/?file=1314429#file1314429line76>
> >
> >     Can you add a comment as to what does this do?

Done.


- Swapan


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


On March 24, 2016, 11:24 p.m., Swapan Shridhar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45325/
> -----------------------------------------------------------
> 
> (Updated March 24, 2016, 11:24 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jaimin Jetly, and Sumit 
> Mohanty.
> 
> 
> Bugs: AMBARI-15573
>     https://issues.apache.org/jira/browse/AMBARI-15573
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Add the Config, Start and Stop logic for: (1). Hive Server Interactive (HSI), 
> and (2). Associate the LLAP lifecycle to it.
> 
> - Start of HSI : Create LLAP package, start LLAP and then start HSI.
> - Stop: Stop HSI and then LLAP.
> 
> 
> Diffs
> -----
> 
>   
> ambari-common/src/main/python/resource_management/libraries/functions/copy_tarball.py
>  f07b76f 
>   
> ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/hive_interactive.py
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/hive_server_interactive.py
>  8a4e7e6 
>   
> ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/hive_service.py
>  12cf336 
>   
> ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/hive_service_interactive.py
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/params_linux.py
>  6f05fbc 
>   
> ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/status_params.py
>  62fc2e3 
>   
> ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/templates/startHiveserver2Interactive.sh.j2
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.0.6/properties/tarball_map.json 
> 1e349a1 
>   
> ambari-server/src/main/resources/stacks/HDP/2.6/services/HIVE/configuration/hive-interactive-site.xml
>  face571 
> 
> Diff: https://reviews.apache.org/r/45325/diff/
> 
> 
> Testing
> -------
> 
> Yes.
>  - 2.6: Installation, config, Start and Stop of Hive Server Interactive after 
> Hive Server Batch -> Works.
>  - 2.6 : Instllation, config, Start and Stop of Hive Server Batch only -> 
> Works
>  - 2.6 : nstallation, config, Start and Stop of Hive Server Interactive along 
> with Hive Server Batch -> Works.
>  - 2.5 : Negative testing :  Only Hive Batch gets installed. (Hive Server 
> Interactive not present).
>  - 
>  - Python UT : Passes.
>  - mvn clean test : ongoing. Will update.
> 
> 
> Thanks,
> 
> Swapan Shridhar
> 
>

Reply via email to