You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Jonathan Hurley <jh...@hortonworks.com> on 2017/01/17 02:54:04 UTC
Review Request 55597: Add upgrade logic for the heap dump control
option added in HDP 2.6 stack.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55597/
-----------------------------------------------------------
Review request for Ambari, Dmitro Lisnichenko, Nate Cole, and Robert Levas.
Bugs: AMBARI-19574
https://issues.apache.org/jira/browse/AMBARI-19574
Repository: ambari
Description
-------
There's a fuller description in the Jira issue, but just to some up for easy of reading:
- Hive added some properties in HDP 2.6
- To upgrade these properties, existing content would need to be modified. Unfortunately, find/replace isn't an option here
- I created a very simple <insert/> directive to prepend/append properties
Diffs
-----
ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java 97280ee
ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java 5428ea7
ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java d7bb338
ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/config-upgrade.xml 40052d8
ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/nonrolling-upgrade-2.6.xml 09608a0
ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.6.xml 949a174
ambari-server/src/main/resources/upgrade-config.xsd e274451
ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 92fa084
Diff: https://reviews.apache.org/r/55597/diff/
Testing
-------
Tests run: 4857, Failures: 0, Errors: 0, Skipped: 38
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 26:49 min
[INFO] Finished at: 2017-01-16T20:53:37-05:00
[INFO] Final Memory: 55M/708M
[INFO] ------------------------------------------------------------------------
Thanks,
Jonathan Hurley
Re: Review Request 55597: Add upgrade logic for the heap dump control
option added in HDP 2.6 stack.
Posted by Alejandro Fernandez <af...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55597/#review161920
-----------------------------------------------------------
Ship it!
Ship It!
- Alejandro Fernandez
On Jan. 17, 2017, 2:54 a.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55597/
> -----------------------------------------------------------
>
> (Updated Jan. 17, 2017, 2:54 a.m.)
>
>
> Review request for Ambari, Dmitro Lisnichenko, Nate Cole, and Robert Levas.
>
>
> Bugs: AMBARI-19574
> https://issues.apache.org/jira/browse/AMBARI-19574
>
>
> Repository: ambari
>
>
> Description
> -------
>
> There's a fuller description in the Jira issue, but just to sum up for easy of reading:
>
> - Hive added some properties in HDP 2.6
> - To upgrade these properties, existing content would need to be modified. Unfortunately, find/replace isn't an option here
> - I created a very simple <insert/> directive to prepend/append properties
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java 97280ee
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java 5428ea7
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java d7bb338
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/config-upgrade.xml 40052d8
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/nonrolling-upgrade-2.6.xml 09608a0
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.6.xml 949a174
> ambari-server/src/main/resources/upgrade-config.xsd e274451
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 92fa084
>
> Diff: https://reviews.apache.org/r/55597/diff/
>
>
> Testing
> -------
>
> Tests run: 4857, Failures: 0, Errors: 0, Skipped: 38
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 26:49 min
> [INFO] Finished at: 2017-01-16T20:53:37-05:00
> [INFO] Final Memory: 55M/708M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 55597: Add upgrade logic for the heap dump control
option added in HDP 2.6 stack.
Posted by Robert Levas <rl...@hortonworks.com>.
> On Jan. 17, 2017, 9:47 a.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java, line 479
> > <https://reviews.apache.org/r/55597/diff/1/?file=1606295#file1606295line479>
> >
> > `contains` seems to be a little relaxed here. What if we are inserting FOO into a value where FOOBAR exists. FOO will not be inserted.
>
> Jonathan Hurley wrote:
> Relaxed, yes, but I think that's why this never made it into Ambari in previous versions. Because questions like this exposed a ton of weird edge cases. And then we end up without the ability to insert content at all.
>
> This was meant as a first attempt to get the new properties in - I can open another Jira for tracking this since I don't see a good solution currently. Would that be sufficient?
>
> Jonathan Hurley wrote:
> Do you think a "beginsWith" and "endsWith" would be sufficient here? Since we're currently only prepending/appending, would that make sense?
>
> Robert Levas wrote:
> I suppopse this can be dropped as long as it is known. But maybe a regular expression match to create a search for "whole words" - ala IntelliJ's "Whole words only" search feature.
>
> Example:
> ```
> \bFOO\b
> ```
>
> Jonathan Hurley wrote:
> I thought of that, but consider this exact case:
>
> foo
> changes to
> foo{{heap_options}}
>
> Now, {{heap_options}} would get re-appended since the whole word match won't work. I don't fully understand why this property was created without a space, but the original committer went to great lengths to add spaces in the Python code whenever they use it. It would have made more sense to me to create it as "foo {{heap_opts}}" .. but alas, it is what it is.
>
> With that said, yes, I can open a Jira to investigate better ways to match to make this idempotent. Right now, it's matching on `contains()` which should be sufficient for most property inserts.
Seems like a no-win secenario... I will drop this but maybe the future will yield a new replace mechanism to handle the case you specified above.. which is different than thia insert case
- Robert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55597/#review161868
-----------------------------------------------------------
On Jan. 16, 2017, 9:54 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55597/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2017, 9:54 p.m.)
>
>
> Review request for Ambari, Dmitro Lisnichenko, Nate Cole, and Robert Levas.
>
>
> Bugs: AMBARI-19574
> https://issues.apache.org/jira/browse/AMBARI-19574
>
>
> Repository: ambari
>
>
> Description
> -------
>
> There's a fuller description in the Jira issue, but just to sum up for easy of reading:
>
> - Hive added some properties in HDP 2.6
> - To upgrade these properties, existing content would need to be modified. Unfortunately, find/replace isn't an option here
> - I created a very simple <insert/> directive to prepend/append properties
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java 97280ee
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java 5428ea7
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java d7bb338
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/config-upgrade.xml 40052d8
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/nonrolling-upgrade-2.6.xml 09608a0
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.6.xml 949a174
> ambari-server/src/main/resources/upgrade-config.xsd e274451
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 92fa084
>
> Diff: https://reviews.apache.org/r/55597/diff/
>
>
> Testing
> -------
>
> Tests run: 4857, Failures: 0, Errors: 0, Skipped: 38
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 26:49 min
> [INFO] Finished at: 2017-01-16T20:53:37-05:00
> [INFO] Final Memory: 55M/708M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 55597: Add upgrade logic for the heap dump control
option added in HDP 2.6 stack.
Posted by Robert Levas <rl...@hortonworks.com>.
> On Jan. 17, 2017, 9:47 a.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java, line 479
> > <https://reviews.apache.org/r/55597/diff/1/?file=1606295#file1606295line479>
> >
> > `contains` seems to be a little relaxed here. What if we are inserting FOO into a value where FOOBAR exists. FOO will not be inserted.
>
> Jonathan Hurley wrote:
> Relaxed, yes, but I think that's why this never made it into Ambari in previous versions. Because questions like this exposed a ton of weird edge cases. And then we end up without the ability to insert content at all.
>
> This was meant as a first attempt to get the new properties in - I can open another Jira for tracking this since I don't see a good solution currently. Would that be sufficient?
>
> Jonathan Hurley wrote:
> Do you think a "beginsWith" and "endsWith" would be sufficient here? Since we're currently only prepending/appending, would that make sense?
I suppopse this can be dropped as long as it is known. But maybe a regular expression match to create a search for "whole words" - ala IntelliJ's "Whole words only" search feature.
Example:
```
\bFOO\b
```
> On Jan. 17, 2017, 9:47 a.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java, lines 488-491
> > <https://reviews.apache.org/r/55597/diff/1/?file=1606295#file1606295line488>
> >
> > Does it make sense to _prepend_ a value where the line separator comes before the value?
> >
> > ```
> > \nFOO + BAR\nBAZ = \nFOOBAR\nBAZ
> > ```
> >
> > (Or maybe I am not interpreting this correctly)
>
> Jonathan Hurley wrote:
> Does it make sense? Perhaps not - but it's an option and if that's what is selected, then we'll do it. There may be a case for it depending on how 2 files might be merged together by some service. For example, if it combines two shell script templates into 1 - you might want that extra newline.
I should have dropped this after I realized what the usecase for this feature is. Dropping now.
> On Jan. 17, 2017, 9:47 a.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java, lines 493-496
> > <https://reviews.apache.org/r/55597/diff/1/?file=1606295#file1606295line493>
> >
> > Does it make sense to _append_ a value where the line separator comes after the value?
> >
> > ```
> > BAR\nBAZ + FOO\n = BAR\nBAZFOO\n
> > ```
> >
> > (Or maybe I am not interpreting this correctly)
>
> Jonathan Hurley wrote:
> I definitely think it does - Git warns you if you don't have a trailing newline in new files. Also, some editors like vi might put the text too low, so developers like adding an extra newline :)
I should have dropped this after I realized what the usecase for this feature is. Dropping now.
- Robert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55597/#review161868
-----------------------------------------------------------
On Jan. 16, 2017, 9:54 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55597/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2017, 9:54 p.m.)
>
>
> Review request for Ambari, Dmitro Lisnichenko, Nate Cole, and Robert Levas.
>
>
> Bugs: AMBARI-19574
> https://issues.apache.org/jira/browse/AMBARI-19574
>
>
> Repository: ambari
>
>
> Description
> -------
>
> There's a fuller description in the Jira issue, but just to sum up for easy of reading:
>
> - Hive added some properties in HDP 2.6
> - To upgrade these properties, existing content would need to be modified. Unfortunately, find/replace isn't an option here
> - I created a very simple <insert/> directive to prepend/append properties
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java 97280ee
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java 5428ea7
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java d7bb338
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/config-upgrade.xml 40052d8
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/nonrolling-upgrade-2.6.xml 09608a0
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.6.xml 949a174
> ambari-server/src/main/resources/upgrade-config.xsd e274451
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 92fa084
>
> Diff: https://reviews.apache.org/r/55597/diff/
>
>
> Testing
> -------
>
> Tests run: 4857, Failures: 0, Errors: 0, Skipped: 38
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 26:49 min
> [INFO] Finished at: 2017-01-16T20:53:37-05:00
> [INFO] Final Memory: 55M/708M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 55597: Add upgrade logic for the heap dump control
option added in HDP 2.6 stack.
Posted by Jonathan Hurley <jh...@hortonworks.com>.
> On Jan. 17, 2017, 9:47 a.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java, line 479
> > <https://reviews.apache.org/r/55597/diff/1/?file=1606295#file1606295line479>
> >
> > `contains` seems to be a little relaxed here. What if we are inserting FOO into a value where FOOBAR exists. FOO will not be inserted.
>
> Jonathan Hurley wrote:
> Relaxed, yes, but I think that's why this never made it into Ambari in previous versions. Because questions like this exposed a ton of weird edge cases. And then we end up without the ability to insert content at all.
>
> This was meant as a first attempt to get the new properties in - I can open another Jira for tracking this since I don't see a good solution currently. Would that be sufficient?
>
> Jonathan Hurley wrote:
> Do you think a "beginsWith" and "endsWith" would be sufficient here? Since we're currently only prepending/appending, would that make sense?
>
> Robert Levas wrote:
> I suppopse this can be dropped as long as it is known. But maybe a regular expression match to create a search for "whole words" - ala IntelliJ's "Whole words only" search feature.
>
> Example:
> ```
> \bFOO\b
> ```
I thought of that, but consider this exact case:
foo
changes to
foo{{heap_options}}
Now, {{heap_options}} would get re-appended since the whole word match won't work. I don't fully understand why this property was created without a space, but the original committer went to great lengths to add spaces in the Python code whenever they use it. It would have made more sense to me to create it as "foo {{heap_opts}}" .. but alas, it is what it is.
With that said, yes, I can open a Jira to investigate better ways to match to make this idempotent. Right now, it's matching on `contains()` which should be sufficient for most property inserts.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55597/#review161868
-----------------------------------------------------------
On Jan. 16, 2017, 9:54 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55597/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2017, 9:54 p.m.)
>
>
> Review request for Ambari, Dmitro Lisnichenko, Nate Cole, and Robert Levas.
>
>
> Bugs: AMBARI-19574
> https://issues.apache.org/jira/browse/AMBARI-19574
>
>
> Repository: ambari
>
>
> Description
> -------
>
> There's a fuller description in the Jira issue, but just to sum up for easy of reading:
>
> - Hive added some properties in HDP 2.6
> - To upgrade these properties, existing content would need to be modified. Unfortunately, find/replace isn't an option here
> - I created a very simple <insert/> directive to prepend/append properties
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java 97280ee
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java 5428ea7
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java d7bb338
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/config-upgrade.xml 40052d8
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/nonrolling-upgrade-2.6.xml 09608a0
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.6.xml 949a174
> ambari-server/src/main/resources/upgrade-config.xsd e274451
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 92fa084
>
> Diff: https://reviews.apache.org/r/55597/diff/
>
>
> Testing
> -------
>
> Tests run: 4857, Failures: 0, Errors: 0, Skipped: 38
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 26:49 min
> [INFO] Finished at: 2017-01-16T20:53:37-05:00
> [INFO] Final Memory: 55M/708M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 55597: Add upgrade logic for the heap dump control
option added in HDP 2.6 stack.
Posted by Jonathan Hurley <jh...@hortonworks.com>.
> On Jan. 17, 2017, 9:47 a.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java, line 479
> > <https://reviews.apache.org/r/55597/diff/1/?file=1606295#file1606295line479>
> >
> > `contains` seems to be a little relaxed here. What if we are inserting FOO into a value where FOOBAR exists. FOO will not be inserted.
>
> Jonathan Hurley wrote:
> Relaxed, yes, but I think that's why this never made it into Ambari in previous versions. Because questions like this exposed a ton of weird edge cases. And then we end up without the ability to insert content at all.
>
> This was meant as a first attempt to get the new properties in - I can open another Jira for tracking this since I don't see a good solution currently. Would that be sufficient?
Do you think a "beginsWith" and "endsWith" would be sufficient here? Since we're currently only prepending/appending, would that make sense?
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55597/#review161868
-----------------------------------------------------------
On Jan. 16, 2017, 9:54 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55597/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2017, 9:54 p.m.)
>
>
> Review request for Ambari, Dmitro Lisnichenko, Nate Cole, and Robert Levas.
>
>
> Bugs: AMBARI-19574
> https://issues.apache.org/jira/browse/AMBARI-19574
>
>
> Repository: ambari
>
>
> Description
> -------
>
> There's a fuller description in the Jira issue, but just to sum up for easy of reading:
>
> - Hive added some properties in HDP 2.6
> - To upgrade these properties, existing content would need to be modified. Unfortunately, find/replace isn't an option here
> - I created a very simple <insert/> directive to prepend/append properties
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java 97280ee
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java 5428ea7
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java d7bb338
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/config-upgrade.xml 40052d8
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/nonrolling-upgrade-2.6.xml 09608a0
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.6.xml 949a174
> ambari-server/src/main/resources/upgrade-config.xsd e274451
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 92fa084
>
> Diff: https://reviews.apache.org/r/55597/diff/
>
>
> Testing
> -------
>
> Tests run: 4857, Failures: 0, Errors: 0, Skipped: 38
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 26:49 min
> [INFO] Finished at: 2017-01-16T20:53:37-05:00
> [INFO] Final Memory: 55M/708M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 55597: Add upgrade logic for the heap dump control
option added in HDP 2.6 stack.
Posted by Jonathan Hurley <jh...@hortonworks.com>.
> On Jan. 17, 2017, 9:47 a.m., Robert Levas wrote:
> > The implementation accounts only for adding content to a line-delimited string value. Would there be a need to insert a value into a string with a different delimiater? Maybe a comma (,)?
>
> Jonathan Hurley wrote:
> No, I don't think so. Consider this:
> `<insert key="llap_java_opts" value="{{heap_dump_opts}}" insert-type="append" newline-before="false" newline-after="false" />`
>
> This will change
> llap_java_opts=-Dfoo -Dbar
> into
> llap_java_opts=-Dfoo -Dbar {{heap_dump_opts}}
>
> Doesn't require any line-delimiting.
Ah, I also see what you're saying now ... That when we do put in a delimiter, we're using a newline as the character. Perhaps if the need arises in the future, we can certainly define the character to use. However, as with all upgrade pack directives, we'd rather start small and only add functionality when it's needed.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55597/#review161868
-----------------------------------------------------------
On Jan. 16, 2017, 9:54 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55597/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2017, 9:54 p.m.)
>
>
> Review request for Ambari, Dmitro Lisnichenko, Nate Cole, and Robert Levas.
>
>
> Bugs: AMBARI-19574
> https://issues.apache.org/jira/browse/AMBARI-19574
>
>
> Repository: ambari
>
>
> Description
> -------
>
> There's a fuller description in the Jira issue, but just to sum up for easy of reading:
>
> - Hive added some properties in HDP 2.6
> - To upgrade these properties, existing content would need to be modified. Unfortunately, find/replace isn't an option here
> - I created a very simple <insert/> directive to prepend/append properties
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java 97280ee
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java 5428ea7
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java d7bb338
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/config-upgrade.xml 40052d8
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/nonrolling-upgrade-2.6.xml 09608a0
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.6.xml 949a174
> ambari-server/src/main/resources/upgrade-config.xsd e274451
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 92fa084
>
> Diff: https://reviews.apache.org/r/55597/diff/
>
>
> Testing
> -------
>
> Tests run: 4857, Failures: 0, Errors: 0, Skipped: 38
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 26:49 min
> [INFO] Finished at: 2017-01-16T20:53:37-05:00
> [INFO] Final Memory: 55M/708M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 55597: Add upgrade logic for the heap dump control
option added in HDP 2.6 stack.
Posted by Jonathan Hurley <jh...@hortonworks.com>.
> On Jan. 17, 2017, 9:47 a.m., Robert Levas wrote:
> > The implementation accounts only for adding content to a line-delimited string value. Would there be a need to insert a value into a string with a different delimiater? Maybe a comma (,)?
No, I don't think so. Consider this:
`<insert key="llap_java_opts" value="{{heap_dump_opts}}" insert-type="append" newline-before="false" newline-after="false" />`
This will change
llap_java_opts=-Dfoo -Dbar
into
llap_java_opts=-Dfoo -Dbar {{heap_dump_opts}}
Doesn't require any line-delimiting.
> On Jan. 17, 2017, 9:47 a.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java, lines 488-491
> > <https://reviews.apache.org/r/55597/diff/1/?file=1606295#file1606295line488>
> >
> > Does it make sense to _prepend_ a value where the line separator comes before the value?
> >
> > ```
> > \nFOO + BAR\nBAZ = \nFOOBAR\nBAZ
> > ```
> >
> > (Or maybe I am not interpreting this correctly)
Does it make sense? Perhaps not - but it's an option and if that's what is selected, then we'll do it. There may be a case for it depending on how 2 files might be merged together by some service. For example, if it combines two shell script templates into 1 - you might want that extra newline.
> On Jan. 17, 2017, 9:47 a.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java, lines 493-496
> > <https://reviews.apache.org/r/55597/diff/1/?file=1606295#file1606295line493>
> >
> > Does it make sense to _append_ a value where the line separator comes after the value?
> >
> > ```
> > BAR\nBAZ + FOO\n = BAR\nBAZFOO\n
> > ```
> >
> > (Or maybe I am not interpreting this correctly)
I definitely think it does - Git warns you if you don't have a trailing newline in new files. Also, some editors like vi might put the text too low, so developers like adding an extra newline :)
> On Jan. 17, 2017, 9:47 a.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java, line 479
> > <https://reviews.apache.org/r/55597/diff/1/?file=1606295#file1606295line479>
> >
> > `contains` seems to be a little relaxed here. What if we are inserting FOO into a value where FOOBAR exists. FOO will not be inserted.
Relaxed, yes, but I think that's why this never made it into Ambari in previous versions. Because questions like this exposed a ton of weird edge cases. And then we end up without the ability to insert content at all.
This was meant as a first attempt to get the new properties in - I can open another Jira for tracking this since I don't see a good solution currently. Would that be sufficient?
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55597/#review161868
-----------------------------------------------------------
On Jan. 16, 2017, 9:54 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55597/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2017, 9:54 p.m.)
>
>
> Review request for Ambari, Dmitro Lisnichenko, Nate Cole, and Robert Levas.
>
>
> Bugs: AMBARI-19574
> https://issues.apache.org/jira/browse/AMBARI-19574
>
>
> Repository: ambari
>
>
> Description
> -------
>
> There's a fuller description in the Jira issue, but just to sum up for easy of reading:
>
> - Hive added some properties in HDP 2.6
> - To upgrade these properties, existing content would need to be modified. Unfortunately, find/replace isn't an option here
> - I created a very simple <insert/> directive to prepend/append properties
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java 97280ee
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java 5428ea7
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java d7bb338
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/config-upgrade.xml 40052d8
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/nonrolling-upgrade-2.6.xml 09608a0
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.6.xml 949a174
> ambari-server/src/main/resources/upgrade-config.xsd e274451
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 92fa084
>
> Diff: https://reviews.apache.org/r/55597/diff/
>
>
> Testing
> -------
>
> Tests run: 4857, Failures: 0, Errors: 0, Skipped: 38
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 26:49 min
> [INFO] Finished at: 2017-01-16T20:53:37-05:00
> [INFO] Final Memory: 55M/708M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 55597: Add upgrade logic for the heap dump control
option added in HDP 2.6 stack.
Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55597/#review161868
-----------------------------------------------------------
Fix it, then Ship it!
The implementation accounts only for adding content to a line-delimited string value. Would there be a need to insert a value into a string with a different delimiater? Maybe a comma (,)?
ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java (line 476)
<https://reviews.apache.org/r/55597/#comment233145>
`contains` seems to be a little relaxed here. What if we are inserting FOO into a value where FOOBAR exists. FOO will not be inserted.
ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java (lines 485 - 488)
<https://reviews.apache.org/r/55597/#comment233146>
Does it make sense to _prepend_ a value where the line separator comes before the value?
```
\nFOO + BAR\nBAZ = \nFOOBAR\nBAZ
```
(Or maybe I am not interpreting this correctly)
ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java (lines 490 - 493)
<https://reviews.apache.org/r/55597/#comment233147>
Does it make sense to _append_ a value where the line separator comes after the value?
```
BAR\nBAZ + FOO\n = BAR\nBAZFOO\n
```
(Or maybe I am not interpreting this correctly)
- Robert Levas
On Jan. 16, 2017, 9:54 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55597/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2017, 9:54 p.m.)
>
>
> Review request for Ambari, Dmitro Lisnichenko, Nate Cole, and Robert Levas.
>
>
> Bugs: AMBARI-19574
> https://issues.apache.org/jira/browse/AMBARI-19574
>
>
> Repository: ambari
>
>
> Description
> -------
>
> There's a fuller description in the Jira issue, but just to sum up for easy of reading:
>
> - Hive added some properties in HDP 2.6
> - To upgrade these properties, existing content would need to be modified. Unfortunately, find/replace isn't an option here
> - I created a very simple <insert/> directive to prepend/append properties
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java 97280ee
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java 5428ea7
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java d7bb338
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/config-upgrade.xml 40052d8
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/nonrolling-upgrade-2.6.xml 09608a0
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.6.xml 949a174
> ambari-server/src/main/resources/upgrade-config.xsd e274451
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 92fa084
>
> Diff: https://reviews.apache.org/r/55597/diff/
>
>
> Testing
> -------
>
> Tests run: 4857, Failures: 0, Errors: 0, Skipped: 38
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 26:49 min
> [INFO] Finished at: 2017-01-16T20:53:37-05:00
> [INFO] Final Memory: 55M/708M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 55597: Add upgrade logic for the heap dump control
option added in HDP 2.6 stack.
Posted by Jonathan Hurley <jh...@hortonworks.com>.
> On Jan. 17, 2017, 2:13 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java, line 498
> > <https://reviews.apache.org/r/55597/diff/1/?file=1606295#file1606295line498>
> >
> > What do you think about checking if valueToInsert is already present in the config?
> >
> > If we want to be more strict, we can check during APPEND that it doesn't already exist at the end, and during PREPEND that it doesn't already exist at the beginning.
> >
> > This should help in cases where the existing config value may already contain in, and to make this idempotent.
We already check for whether it exists in the config. Rob was saying that it was too restrictive, so I proposed that we could check beginsWith or endsWith ... but that can be defeated easily with `content` sections since people add to those all the time. That's why I'd rather worry about the edge cases in a separate Jira.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55597/#review161911
-----------------------------------------------------------
On Jan. 16, 2017, 9:54 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55597/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2017, 9:54 p.m.)
>
>
> Review request for Ambari, Dmitro Lisnichenko, Nate Cole, and Robert Levas.
>
>
> Bugs: AMBARI-19574
> https://issues.apache.org/jira/browse/AMBARI-19574
>
>
> Repository: ambari
>
>
> Description
> -------
>
> There's a fuller description in the Jira issue, but just to sum up for easy of reading:
>
> - Hive added some properties in HDP 2.6
> - To upgrade these properties, existing content would need to be modified. Unfortunately, find/replace isn't an option here
> - I created a very simple <insert/> directive to prepend/append properties
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java 97280ee
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java 5428ea7
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java d7bb338
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/config-upgrade.xml 40052d8
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/nonrolling-upgrade-2.6.xml 09608a0
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.6.xml 949a174
> ambari-server/src/main/resources/upgrade-config.xsd e274451
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 92fa084
>
> Diff: https://reviews.apache.org/r/55597/diff/
>
>
> Testing
> -------
>
> Tests run: 4857, Failures: 0, Errors: 0, Skipped: 38
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 26:49 min
> [INFO] Finished at: 2017-01-16T20:53:37-05:00
> [INFO] Final Memory: 55M/708M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 55597: Add upgrade logic for the heap dump control
option added in HDP 2.6 stack.
Posted by Alejandro Fernandez <af...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55597/#review161911
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java (line 495)
<https://reviews.apache.org/r/55597/#comment233196>
What do you think about checking if valueToInsert is already present in the config?
If we want to be more strict, we can check during APPEND that it doesn't already exist at the end, and during PREPEND that it doesn't already exist at the beginning.
This should help in cases where the existing config value may already contain in, and to make this idempotent.
- Alejandro Fernandez
On Jan. 17, 2017, 2:54 a.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55597/
> -----------------------------------------------------------
>
> (Updated Jan. 17, 2017, 2:54 a.m.)
>
>
> Review request for Ambari, Dmitro Lisnichenko, Nate Cole, and Robert Levas.
>
>
> Bugs: AMBARI-19574
> https://issues.apache.org/jira/browse/AMBARI-19574
>
>
> Repository: ambari
>
>
> Description
> -------
>
> There's a fuller description in the Jira issue, but just to sum up for easy of reading:
>
> - Hive added some properties in HDP 2.6
> - To upgrade these properties, existing content would need to be modified. Unfortunately, find/replace isn't an option here
> - I created a very simple <insert/> directive to prepend/append properties
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java 97280ee
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java 5428ea7
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java d7bb338
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/config-upgrade.xml 40052d8
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/nonrolling-upgrade-2.6.xml 09608a0
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.6.xml 949a174
> ambari-server/src/main/resources/upgrade-config.xsd e274451
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 92fa084
>
> Diff: https://reviews.apache.org/r/55597/diff/
>
>
> Testing
> -------
>
> Tests run: 4857, Failures: 0, Errors: 0, Skipped: 38
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 26:49 min
> [INFO] Finished at: 2017-01-16T20:53:37-05:00
> [INFO] Final Memory: 55M/708M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 55597: Add upgrade logic for the heap dump control
option added in HDP 2.6 stack.
Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55597/#review161923
-----------------------------------------------------------
Ship it!
Ship It!
- Nate Cole
On Jan. 16, 2017, 9:54 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55597/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2017, 9:54 p.m.)
>
>
> Review request for Ambari, Dmitro Lisnichenko, Nate Cole, and Robert Levas.
>
>
> Bugs: AMBARI-19574
> https://issues.apache.org/jira/browse/AMBARI-19574
>
>
> Repository: ambari
>
>
> Description
> -------
>
> There's a fuller description in the Jira issue, but just to sum up for easy of reading:
>
> - Hive added some properties in HDP 2.6
> - To upgrade these properties, existing content would need to be modified. Unfortunately, find/replace isn't an option here
> - I created a very simple <insert/> directive to prepend/append properties
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java 97280ee
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java 5428ea7
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java d7bb338
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/config-upgrade.xml 40052d8
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/nonrolling-upgrade-2.6.xml 09608a0
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.6.xml 949a174
> ambari-server/src/main/resources/upgrade-config.xsd e274451
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 92fa084
>
> Diff: https://reviews.apache.org/r/55597/diff/
>
>
> Testing
> -------
>
> Tests run: 4857, Failures: 0, Errors: 0, Skipped: 38
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 26:49 min
> [INFO] Finished at: 2017-01-16T20:53:37-05:00
> [INFO] Final Memory: 55M/708M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 55597: Add upgrade logic for the heap dump control
option added in HDP 2.6 stack.
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55597/#review161811
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java (lines 469 - 506)
<https://reviews.apache.org/r/55597/#comment233117>
This is really the bulk of this review. Take the Insert instances and apply prepend/append logic. The work should be idempotent; I accomplish this simply by checking for whether the property already contains the new content. I know it's not perfect, but it'll satisfy every upgrade story we have so far.
- Jonathan Hurley
On Jan. 16, 2017, 9:54 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55597/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2017, 9:54 p.m.)
>
>
> Review request for Ambari, Dmitro Lisnichenko, Nate Cole, and Robert Levas.
>
>
> Bugs: AMBARI-19574
> https://issues.apache.org/jira/browse/AMBARI-19574
>
>
> Repository: ambari
>
>
> Description
> -------
>
> There's a fuller description in the Jira issue, but just to sum up for easy of reading:
>
> - Hive added some properties in HDP 2.6
> - To upgrade these properties, existing content would need to be modified. Unfortunately, find/replace isn't an option here
> - I created a very simple <insert/> directive to prepend/append properties
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java 97280ee
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java 5428ea7
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java d7bb338
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/config-upgrade.xml 40052d8
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/nonrolling-upgrade-2.6.xml 09608a0
> ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.6.xml 949a174
> ambari-server/src/main/resources/upgrade-config.xsd e274451
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 92fa084
>
> Diff: https://reviews.apache.org/r/55597/diff/
>
>
> Testing
> -------
>
> Tests run: 4857, Failures: 0, Errors: 0, Skipped: 38
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 26:49 min
> [INFO] Finished at: 2017-01-16T20:53:37-05:00
> [INFO] Final Memory: 55M/708M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 55597: Add upgrade logic for the heap dump control
option added in HDP 2.6 stack.
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55597/
-----------------------------------------------------------
(Updated Jan. 16, 2017, 9:54 p.m.)
Review request for Ambari, Dmitro Lisnichenko, Nate Cole, and Robert Levas.
Bugs: AMBARI-19574
https://issues.apache.org/jira/browse/AMBARI-19574
Repository: ambari
Description (updated)
-------
There's a fuller description in the Jira issue, but just to sum up for easy of reading:
- Hive added some properties in HDP 2.6
- To upgrade these properties, existing content would need to be modified. Unfortunately, find/replace isn't an option here
- I created a very simple <insert/> directive to prepend/append properties
Diffs
-----
ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java 97280ee
ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java 5428ea7
ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java d7bb338
ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/config-upgrade.xml 40052d8
ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/nonrolling-upgrade-2.6.xml 09608a0
ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.6.xml 949a174
ambari-server/src/main/resources/upgrade-config.xsd e274451
ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 92fa084
Diff: https://reviews.apache.org/r/55597/diff/
Testing
-------
Tests run: 4857, Failures: 0, Errors: 0, Skipped: 38
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 26:49 min
[INFO] Finished at: 2017-01-16T20:53:37-05:00
[INFO] Final Memory: 55M/708M
[INFO] ------------------------------------------------------------------------
Thanks,
Jonathan Hurley