[Impala-ASF-CR] IMPALA-4653: fix sticky config variable problem

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4653: fix sticky config variable problem
..


Patch Set 3: Verified+1

-- 
To view, visit http://gerrit.cloudera.org:8080/5545
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I930e2ca825142428d17a6981c77534ab0c8e3489
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4653: fix sticky config variable problem

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-4653: fix sticky config variable problem
..


IMPALA-4653: fix sticky config variable problem

Previously we could get a developer's shell into a bad state where a
value of a config variable from a previous impala-config.sh version
would override the value from the new impala-config.sh version.

This change adds a new mechanism to override settings locally by adding
settings to impala-config-local.sh. This alternative approach is more
robust, because the config variables will be reset to the intended
values when impala-config.sh is re-sourced.

impala-config-branch.sh can also be used to override settings in a
version-controlled way, e.g. to support having different settings for
different branches.

I did not convert all variables to use this approach, since many people
and Jenkins jobs depend on setting these variables from the environment.
The remaining "sticky" variables are ones where default values should
not change frequently, e.g. source directory locations and build
settings.

Change-Id: I930e2ca825142428d17a6981c77534ab0c8e3489
Reviewed-on: http://gerrit.cloudera.org:8080/5545
Reviewed-by: Matthew Jacobs 
Tested-by: Impala Public Jenkins
---
M .gitignore
A bin/impala-config-branch.sh
M bin/impala-config.sh
3 files changed, 137 insertions(+), 96 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Matthew Jacobs: Looks good to me, approved



-- 
To view, visit http://gerrit.cloudera.org:8080/5545
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I930e2ca825142428d17a6981c77534ab0c8e3489
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-4653: fix sticky config variable problem

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4653: fix sticky config variable problem
..


Patch Set 3: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/5545
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I930e2ca825142428d17a6981c77534ab0c8e3489
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4653: fix sticky config variable problem

[Tim Armstrong (Code Review)]

Hello Dan Hecht,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/5545

to look at the new patch set (#3).

Change subject: IMPALA-4653: fix sticky config variable problem
..

IMPALA-4653: fix sticky config variable problem

Previously we could get a developer's shell into a bad state where a
value of a config variable from a previous impala-config.sh version
would override the value from the new impala-config.sh version.

This change adds a new mechanism to override settings locally by adding
settings to impala-config-local.sh. This alternative approach is more
robust, because the config variables will be reset to the intended
values when impala-config.sh is re-sourced.

impala-config-branch.sh can also be used to override settings in a
version-controlled way, e.g. to support having different settings for
different branches.

I did not convert all variables to use this approach, since many people
and Jenkins jobs depend on setting these variables from the environment.
The remaining "sticky" variables are ones where default values should
not change frequently, e.g. source directory locations and build
settings.

Change-Id: I930e2ca825142428d17a6981c77534ab0c8e3489
---
M .gitignore
A bin/impala-config-branch.sh
M bin/impala-config.sh
3 files changed, 137 insertions(+), 96 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/5545/3
-- 
To view, visit http://gerrit.cloudera.org:8080/5545
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I930e2ca825142428d17a6981c77534ab0c8e3489
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-4653: fix sticky config variable problem

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4653: fix sticky config variable problem
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5545/2/bin/impala-config-branch.sh
File bin/impala-config-branch.sh:

Line 22
> an example would be helpful (e.g. in a comment) to be clear. I'm not sure w
Done


http://gerrit.cloudera.org:8080/#/c/5545/2/bin/impala-config.sh
File bin/impala-config.sh:

Line 75: IMPALA_TOOLCHAIN_BUILD_ID=308-96a4cc516e
> export this here rather than doing so separately on l179, to be consistent 
I ended up redoing this to be consistent everywhere - it was confusing because 
we use two different . 

The next patchset just exports the variables when they're first declared i.e.

  export FOO=${FOO-default}
  
instead of

   : ${FOO=default}
   export FOO.


-- 
To view, visit http://gerrit.cloudera.org:8080/5545
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I930e2ca825142428d17a6981c77534ab0c8e3489
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4653: fix sticky config variable problem

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4653: fix sticky config variable problem
..


Patch Set 2:

(2 comments)

thanks!

http://gerrit.cloudera.org:8080/#/c/5545/2/bin/impala-config-branch.sh
File bin/impala-config-branch.sh:

Line 22
an example would be helpful (e.g. in a comment) to be clear. I'm not sure 
without looking at some bash documentation if this should have the export or 
not.


http://gerrit.cloudera.org:8080/#/c/5545/2/bin/impala-config.sh
File bin/impala-config.sh:

Line 75: IMPALA_TOOLCHAIN_BUILD_ID=308-96a4cc516e
export this here rather than doing so separately on l179, to be consistent with 
the other variables below which can be overridden.


-- 
To view, visit http://gerrit.cloudera.org:8080/5545
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I930e2ca825142428d17a6981c77534ab0c8e3489
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4653: fix sticky config variable problem

[Tim Armstrong (Code Review)]

Hello Dan Hecht,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/5545

to look at the new patch set (#2).

Change subject: IMPALA-4653: fix sticky config variable problem
..

IMPALA-4653: fix sticky config variable problem

Previously we could get a developer's shell into a bad state where a
value of a config variable from a previous impala-config.sh version
would override the value from the new impala-config.sh version.

This change adds a new mechanism to override settings locally by adding
settings to impala-config-local.sh. This alternative approach is more
robust, because the config variables will be reset to the intended
values when impala-config.sh is re-sourced.

impala-config-branch.sh can also be used to override settings in a
version-controlled way, e.g. to support having different settings for
different branches.

I did not convert all variables to use this approach, since many people
and Jenkins jobs depend on setting these variables from the environment.
The remaining "sticky" variables are ones where default values should
not change frequently, e.g. source directory locations and build
settings.

Change-Id: I930e2ca825142428d17a6981c77534ab0c8e3489
---
M .gitignore
A bin/impala-config-branch.sh
M bin/impala-config.sh
3 files changed, 116 insertions(+), 59 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/5545/2
-- 
To view, visit http://gerrit.cloudera.org:8080/5545
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I930e2ca825142428d17a6981c77534ab0c8e3489
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 

[Impala-ASF-CR] IMPALA-4653: fix sticky config variable problem

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change.

Change subject: IMPALA-4653: fix sticky config variable problem
..


Patch Set 1: Code-Review+1

Thanks for fixing this!

-- 
To view, visit http://gerrit.cloudera.org:8080/5545
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I930e2ca825142428d17a6981c77534ab0c8e3489
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-HasComments: No