Re: Review Request 57584: RANGER-1449:There is logical inconsistency for RANGER_PID_DIR_PATH in security admin

2017-03-21 Thread Colm O hEigeartaigh 

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


Ship it!




Ship It!

- Colm O hEigeartaigh


On March 21, 2017, 7:10 a.m., pengjianhua wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57584/
> ---
> 
> (Updated March 21, 2017, 7:10 a.m.)
> 
> 
> Review request for ranger, Alok Lal, Ankita Sinha, Don Bosco Durai, Colm O 
> hEigeartaigh, Gautam Borad, Madhan Neethiraj, Ramesh Mani, Selvamohan 
> Neethiraj, Velmurugan Periasamy, and Qiang Zhang.
> 
> 
> Bugs: RANGER-1449
> https://issues.apache.org/jira/browse/RANGER-1449
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> 1. There is RANGER_PID_DIR_PATH parameter in install.properties. But There is 
> hard code at the begin of setup.sh
> pidFolderName='/var/run/ranger'
> mkdir -p $
> {pidFolderName}
> 
> if [ ! $? = "0" ];then
> log "Make $pidFolderName failure!!";
> exit 1;
> fi
> 2. The ranger-admin-services.sh will set default value and create default 
> path when the RANGER_PID_DIR_PATH parameter is empty. The setup.sh doesn't 
> set default value and create default path.
> 
> 
> Diffs
> -
> 
>   security-admin/scripts/setup.sh 87be127 
> 
> 
> Diff: https://reviews.apache.org/r/57584/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> pengjianhua
> 
>

Re: Review Request 57584: RANGER-1449:There is logical inconsistency for RANGER_PID_DIR_PATH in security admin

2017-03-20 Thread Colm O hEigeartaigh 

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



There's just one more change I'd like to see. The if statement "if [ -d 
${RANGER_PID_DIR_PATH} ]; then" containing "chown" is not necessary, as by this 
point we already know that RANGER_PID_DIR_PATH is a directory. So you could 
remove the if statement here, preserving the chown part inside it.

- Colm O hEigeartaigh


On March 17, 2017, 3:29 a.m., pengjianhua wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57584/
> ---
> 
> (Updated March 17, 2017, 3:29 a.m.)
> 
> 
> Review request for ranger, Alok Lal, Ankita Sinha, Don Bosco Durai, Colm O 
> hEigeartaigh, Gautam Borad, Madhan Neethiraj, Ramesh Mani, Selvamohan 
> Neethiraj, Velmurugan Periasamy, and Qiang Zhang.
> 
> 
> Bugs: RANGER-1449
> https://issues.apache.org/jira/browse/RANGER-1449
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> 1. There is RANGER_PID_DIR_PATH parameter in install.properties. But There is 
> hard code at the begin of setup.sh
> pidFolderName='/var/run/ranger'
> mkdir -p $
> {pidFolderName}
> 
> if [ ! $? = "0" ];then
> log "Make $pidFolderName failure!!";
> exit 1;
> fi
> 2. The ranger-admin-services.sh will set default value and create default 
> path when the RANGER_PID_DIR_PATH parameter is empty. The setup.sh doesn't 
> set default value and create default path.
> 
> 
> Diffs
> -
> 
>   security-admin/scripts/setup.sh 87be127 
> 
> 
> Diff: https://reviews.apache.org/r/57584/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> pengjianhua
> 
>