You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hawq.apache.org by interma <gi...@git.apache.org> on 2017/05/09 06:29:31 UTC

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

GitHub user interma opened a pull request:

    https://github.com/apache/incubator-hawq/pull/1235

    HAWQ-1456. Copy RPS configuration files to standby in specific scenarios

    copy RPS configuration files to standby in:
      * hawq init standby
      * enable-ranger-plugin.sh
    
    besides copy files, enhance enable-ranger-plugin.sh to support read items(e.g. hawq_master_address_host) in hawq-site.xml now.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/interma/interma-hawq hawq-1456

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-hawq/pull/1235.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1235
    
----

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115640826
  
    --- Diff: ranger-plugin/scripts/enable-ranger-plugin.sh ---
    @@ -237,9 +305,13 @@ main() {
       if [[ $# -lt 1 ]]; then
         usage
       fi
    +  if [[ -z "$GPHOME" ]]; then
    +    GPHOME="/usr/local/hawq"
    +  fi
       SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd -P)"
       parse_params "$@"
       validate_params
    +  sync_rps_configuration
    --- End diff --
    
    thanks, alex!
    all comments are useful, I will fix them.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by denalex <gi...@git.apache.org>.
Github user denalex commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115550195
  
    --- Diff: ranger-plugin/scripts/enable-ranger-plugin.sh ---
    @@ -104,8 +157,18 @@ function get_hawq_password() {
       done
     }
     
    +function sync_rps_configuration() {
    +  if [[ -d $GPHOME/ranger/etc/ && ! -z $HAWQ_STANDBY_HOST ]]; then
    +    cmd="scp $GPHOME/ranger/etc/* $HAWQ_STANDBY_HOST:$GPHOME/ranger/etc/"
    --- End diff --
    
    why do we need a variable and not run the command directly ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq issue #1235: HAWQ-1456. Copy RPS configuration files to stand...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on the issue:

    https://github.com/apache/incubator-hawq/pull/1235
  
    merged


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by denalex <gi...@git.apache.org>.
Github user denalex commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115549773
  
    --- Diff: ranger-plugin/scripts/enable-ranger-plugin.sh ---
    @@ -104,8 +157,18 @@ function get_hawq_password() {
       done
     }
     
    +function sync_rps_configuration() {
    +  if [[ -d $GPHOME/ranger/etc/ && ! -z $HAWQ_STANDBY_HOST ]]; then
    --- End diff --
    
    I'd print a statement telling user what the script is about to do as well as the status once done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by interma <gi...@git.apache.org>.
Github user interma closed the pull request at:

    https://github.com/apache/incubator-hawq/pull/1235


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by stanlyxiang <gi...@git.apache.org>.
Github user stanlyxiang commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115425328
  
    --- Diff: ranger-plugin/scripts/enable-ranger-plugin.sh ---
    @@ -237,9 +305,13 @@ main() {
       if [[ $# -lt 1 ]]; then
         usage
       fi
    +  if [[ -z "$GPHOME" ]]; then
    +    GPHOME="/usr/local/hawq"
    --- End diff --
    
    what if user not set GPHOME and "/usr/local/hawq" is not the right place ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115450794
  
    --- Diff: ranger-plugin/scripts/enable-ranger-plugin.sh ---
    @@ -104,8 +157,18 @@ function get_hawq_password() {
       done
     }
     
    +function sync_rps_configuration() {
    +  if [[ -d $GPHOME/ranger/etc/ && ! -z $HAWQ_STANDBY_HOST ]]; then
    +    cmd="scp $GPHOME/ranger/etc/* $HAWQ_STANDBY_HOST:$GPHOME/ranger/etc/"
    +    $cmd
    +    if [ $? != 0 ]; then
    +      fail "Scp RPS configuration files to Standby failed."
    --- End diff --
    
    ok, thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by linwen <gi...@git.apache.org>.
Github user linwen commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115417102
  
    --- Diff: ranger-plugin/scripts/enable-ranger-plugin.sh ---
    @@ -104,8 +157,20 @@ function get_hawq_password() {
       done
     }
     
    +function sync_rps_configuration() {
    --- End diff --
    
    should follow 2 spaces indent. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by denalex <gi...@git.apache.org>.
Github user denalex commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115549262
  
    --- Diff: ranger-plugin/scripts/enable-ranger-plugin.sh ---
    @@ -86,6 +119,27 @@ function get_hawq_url() {
       fi
       HAWQ_HOST=${parts[0]}
       HAWQ_PORT=${parts[1]}
    +
    +  # get hawq standby host
    +  # 1. read from command parameter -s
    +  # 2. read from hawq-site.xml
    +  if [[ -z "$HAWQ_STANDBY_HOST" ]]; then
    +    value=''
    +    get_hawq_property hawq_standby_address_host
    +    local host=$value
    +
    +    if [[ "$host" == "none" ]]; then
    +      HAWQ_STANDBY_HOST=''
    +    else
    +      HAWQ_STANDBY_HOST=$host
    +    fi
    +  fi
    +
    +  # 3. read from user input
    +  while [[ -z "$HAWQ_STANDBY_HOST" ]]
    --- End diff --
    
    what if there is no standby host at all, why would we force the user to enter the value here ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115442221
  
    --- Diff: tools/bin/hawq_ctl ---
    @@ -387,6 +387,10 @@ class HawqInit:
                      (self.GPHOME, self.standby_host_name, self.GPHOME)
             check_return_code(remote_ssh(scpcmd, self.master_host_name, self.user), \
                               logger, "Sync slaves file failed")
    +        scpcmd = "scp %s/ranger/etc/* %s:%s/ranger/etc/ > /dev/null" % \
    +                 (self.GPHOME, self.standby_host_name, self.GPHOME)
    +        check_return_code(remote_ssh(scpcmd, self.master_host_name, self.user), \
    +                          logger, "Sync rps configuration files failed")
    --- End diff --
    
    Thanks! I forgot this conditions.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq issue #1235: HAWQ-1456. Copy RPS configuration files to stand...

Posted by linwen <gi...@git.apache.org>.
Github user linwen commented on the issue:

    https://github.com/apache/incubator-hawq/pull/1235
  
    +1 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq issue #1235: HAWQ-1456. Copy RPS configuration files to stand...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on the issue:

    https://github.com/apache/incubator-hawq/pull/1235
  
    @linwen @stanlyxiang @zhangh43 help to review, tks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115644616
  
    --- Diff: ranger-plugin/scripts/enable-ranger-plugin.sh ---
    @@ -104,8 +157,18 @@ function get_hawq_password() {
       done
     }
     
    +function sync_rps_configuration() {
    +  if [[ -d $GPHOME/ranger/etc/ && ! -z $HAWQ_STANDBY_HOST ]]; then
    +    cmd="scp $GPHOME/ranger/etc/* $HAWQ_STANDBY_HOST:$GPHOME/ranger/etc/"
    --- End diff --
    
    Reasonable.
    But there are some other stuffs(rps ha tests,docs) assumed the script should copy rps configuration files to standby...
    I think copy is also ok, and we can finish other stuffs quickly.
    
    In future, we will go back to consider whether remove copy is more easy-to-use for user.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115643220
  
    --- Diff: ranger-plugin/scripts/enable-ranger-plugin.sh ---
    @@ -237,9 +305,13 @@ main() {
       if [[ $# -lt 1 ]]; then
         usage
       fi
    +  if [[ -z "$GPHOME" ]]; then
    +    GPHOME="/usr/local/hawq"
    --- End diff --
    
    You are right, I saw *SCRIPT_DIR*, I will use it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by denalex <gi...@git.apache.org>.
Github user denalex commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115551737
  
    --- Diff: ranger-plugin/scripts/enable-ranger-plugin.sh ---
    @@ -237,9 +305,13 @@ main() {
       if [[ $# -lt 1 ]]; then
         usage
       fi
    +  if [[ -z "$GPHOME" ]]; then
    +    GPHOME="/usr/local/hawq"
    +  fi
       SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd -P)"
       parse_params "$@"
       validate_params
    +  sync_rps_configuration
    --- End diff --
    
    if sync fails, Ranger definitions are not created, which is not good. Service can work without standby and without sync, so either not fail (just warn) or move it to the very last step. Better yet, not to sync at all here, as I commented above.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by stanlyxiang <gi...@git.apache.org>.
Github user stanlyxiang commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115424467
  
    --- Diff: tools/bin/hawq_ctl ---
    @@ -387,6 +387,10 @@ class HawqInit:
                      (self.GPHOME, self.standby_host_name, self.GPHOME)
             check_return_code(remote_ssh(scpcmd, self.master_host_name, self.user), \
                               logger, "Sync slaves file failed")
    +        scpcmd = "scp %s/ranger/etc/* %s:%s/ranger/etc/ > /dev/null" % \
    +                 (self.GPHOME, self.standby_host_name, self.GPHOME)
    +        check_return_code(remote_ssh(scpcmd, self.master_host_name, self.user), \
    +                          logger, "Sync rps configuration files failed")
    --- End diff --
    
    This code snippet is used for copy ranger configuration files to standby when do "hawq init standby" when ranger was configured.  But there maybe no ranger.  Standalone case will fail.
    e.g. When hawq init cluster(it will init standby), hawq is absolutely not configured with ranger. If there is a cluster with hawq master and segment configured with ranger, then we should scp rps configuration files to standby when "hawq init standby".


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by linwen <gi...@git.apache.org>.
Github user linwen commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115417172
  
    --- Diff: ranger-plugin/scripts/enable-ranger-plugin.sh ---
    @@ -237,9 +307,14 @@ main() {
       if [[ $# -lt 1 ]]; then
         usage
       fi
    +  if [[ -z "$GPHOME" ]]; then
    +	GPHOME="/usr/local/hawq"
    +  fi
       SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd -P)"
       parse_params "$@"
       validate_params
    +  sync_rps_configuration
    +  #exit 0
    --- End diff --
    
    useless codes. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq issue #1235: HAWQ-1456. Copy RPS configuration files to stand...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on the issue:

    https://github.com/apache/incubator-hawq/pull/1235
  
    All fixed, help to review again.
    Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by denalex <gi...@git.apache.org>.
Github user denalex commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115551379
  
    --- Diff: ranger-plugin/scripts/enable-ranger-plugin.sh ---
    @@ -237,9 +305,13 @@ main() {
       if [[ $# -lt 1 ]]; then
         usage
       fi
    +  if [[ -z "$GPHOME" ]]; then
    +    GPHOME="/usr/local/hawq"
    --- End diff --
    
    we do not need GPHOME, just use relative path to current script. No reason to ask users to source extra files.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq issue #1235: HAWQ-1456. Copy RPS configuration files to stand...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on the issue:

    https://github.com/apache/incubator-hawq/pull/1235
  
    Removed scp rps files.
    Reserved read property from hawq-site.xml (read hawq_master_address_host and hawq_master_address_port).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by stanlyxiang <gi...@git.apache.org>.
Github user stanlyxiang commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115425270
  
    --- Diff: ranger-plugin/scripts/enable-ranger-plugin.sh ---
    @@ -104,8 +157,18 @@ function get_hawq_password() {
       done
     }
     
    +function sync_rps_configuration() {
    +  if [[ -d $GPHOME/ranger/etc/ && ! -z $HAWQ_STANDBY_HOST ]]; then
    +    cmd="scp $GPHOME/ranger/etc/* $HAWQ_STANDBY_HOST:$GPHOME/ranger/etc/"
    +    $cmd
    +    if [ $? != 0 ]; then
    +      fail "Scp RPS configuration files to Standby failed."
    --- End diff --
    
    "scp" is a command, weird to say "Scp".


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by linwen <gi...@git.apache.org>.
Github user linwen commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115417038
  
    --- Diff: ranger-plugin/scripts/enable-ranger-plugin.sh ---
    @@ -70,14 +70,47 @@ function get_ranger_password() {
       done
     }
     
    +# get tag value from hawq-site.xml
    +function get_value_bytag() {
    --- End diff --
    
    suggest use "property" instead of "tag".


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by denalex <gi...@git.apache.org>.
Github user denalex commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115548918
  
    --- Diff: ranger-plugin/scripts/enable-ranger-plugin.sh ---
    @@ -70,14 +70,47 @@ function get_ranger_password() {
       done
     }
     
    +# get property value from hawq-site.xml
    +function get_hawq_property() {
    +  local hawq_site_file="$GPHOME/etc/hawq-site.xml"
    +  tag=$1
    +
    +  if [ -f $hawq_site_file ] ; then
    +    value=`cat $hawq_site_file | tr '\n' ' ' | awk -F '<property>' '{ for(i = 1; i <= NF; i++) { print $i; } }' | grep $tag | sed -n 's|.*<value>\(.*\)</value>.*|\1|p'`
    +  else
    +    value=''
    +  fi
    +}
    +
     function get_hawq_url() {
    -  #todo read hawq-site.xml ?
    +
    +  # get hawq master host and port
    +  # 1. read from command parameter -h
    +  # 2. read from hawq-site.xml
    +  if [[ -z "$HAWQ_URL" ]]; then
    +    value=''
    +    get_hawq_property hawq_master_address_host
    +    local host=$value
    --- End diff --
    
    if you have function's last line do echo $value, then here you can just say local host = $(func) instead of setting/resetting global variable


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by denalex <gi...@git.apache.org>.
Github user denalex commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115547649
  
    --- Diff: ranger-plugin/scripts/enable-ranger-plugin.sh ---
    @@ -70,14 +70,47 @@ function get_ranger_password() {
       done
     }
     
    +# get property value from hawq-site.xml
    +function get_hawq_property() {
    +  local hawq_site_file="$GPHOME/etc/hawq-site.xml"
    --- End diff --
    
    I would use relative path to the script location instead of relying on GPHOME


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by denalex <gi...@git.apache.org>.
Github user denalex commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115550883
  
    --- Diff: ranger-plugin/scripts/enable-ranger-plugin.sh ---
    @@ -104,8 +157,18 @@ function get_hawq_password() {
       done
     }
     
    +function sync_rps_configuration() {
    +  if [[ -d $GPHOME/ranger/etc/ && ! -z $HAWQ_STANDBY_HOST ]]; then
    +    cmd="scp $GPHOME/ranger/etc/* $HAWQ_STANDBY_HOST:$GPHOME/ranger/etc/"
    --- End diff --
    
    are we sure we want to copy the files here as part of this script ? I'd say it would be better to have it working on a single node only and do any cluster-level operations in "hawq" management scripts. Also, in the future config files will be managed by Ambari. I'd say it even could be ok to ask users to copy the files to standby manually, especially since when failover occurs, they will need to provision a new standby and will have to copy config files there manually anyways.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115422436
  
    --- Diff: ranger-plugin/scripts/enable-ranger-plugin.sh ---
    @@ -237,9 +307,14 @@ main() {
       if [[ $# -lt 1 ]]; then
         usage
       fi
    +  if [[ -z "$GPHOME" ]]; then
    +	GPHOME="/usr/local/hawq"
    +  fi
       SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd -P)"
       parse_params "$@"
       validate_params
    +  sync_rps_configuration
    +  #exit 0
    --- End diff --
    
    all fixed, thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1235: HAWQ-1456. Copy RPS configuration files t...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1235#discussion_r115450646
  
    --- Diff: ranger-plugin/scripts/enable-ranger-plugin.sh ---
    @@ -237,9 +305,13 @@ main() {
       if [[ $# -lt 1 ]]; then
         usage
       fi
    +  if [[ -z "$GPHOME" ]]; then
    +    GPHOME="/usr/local/hawq"
    --- End diff --
    
    when no GPHOME and "/usr/local/hawq", code run here:
    https://github.com/interma/interma-hawq/blob/a49f85566fd136cc7940fdff15d50c985a99d2cc/ranger-plugin/scripts/enable-ranger-plugin.sh#L111
    
    It ask user to input, so it's ok. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---