Re: Review Request 62809: AMBARI-22159. Replace hostgroup vars for Druid

[Attila Doroszlai]

> On Oct. 9, 2017, 4:23 p.m., Sebastian Toader wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
> > Line 1374 (original), 1420 (patched)
> > 
> >
> > Can you do the same for `MultipleHostTopologyUpdater` and other 
> > updaters which currently process hostgroup plcaeholders?

`MultipleHostTopologyUpdater` has much more complex logic for hostgroups, which 
I'd rather not change now.


- Attila


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


On Oct. 9, 2017, 2:43 p.m., Attila Doroszlai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62809/
> ---
> 
> (Updated Oct. 9, 2017, 2:43 p.m.)
> 
> 
> Review request for Ambari, Nishant Bangarwa, Slim Bouguerra, Sandor Magyari, 
> Swapan Shridhar, and Sebastian Toader.
> 
> 
> Bugs: AMBARI-22159
> https://issues.apache.org/jira/browse/AMBARI-22159
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Replace placeholders in `metastore_hostname`, 
> `druid.metadata.storage.connector.connectURI` and `druid.zk.service.host` 
> when creating cluster via blueprint.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
>  5a6e2cc0de7dc0c0f4499624e21c43bbc8d24b54 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
>  68d6349db0207ab6df44b4b0a341c77ecabc46ea 
> 
> 
> Diff: https://reviews.apache.org/r/62809/diff/2/
> 
> 
> Testing
> ---
> 
> Tested manually on local cluster.
> 
> Unit tests pass: `Tests run: 4900, Failures: 0, Errors: 0, Skipped: 36`
> 
> 
> Thanks,
> 
> Attila Doroszlai
> 
>

Re: Review Request 62809: AMBARI-22159. Replace hostgroup vars for Druid

[Robert Nettleton]

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


Ship it!




Overall, the patch looks fine to me, just a comment below about making sure 
this is tested thoroughly, since this patch does introduce a slight change to 
the Blueprints model for updater registration.  

Thanks.


ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
Lines 2812 (patched)


Generally, the model here is to create a new property updater for each 
property.

Off the top of my head, it seems fine to use a singleton for the property 
updater, but please make sure this change is tested thoroughly, since there 
might be un-intended consequences of changing the way these updaters are 
registered.


- Robert Nettleton


On Oct. 9, 2017, 12:43 p.m., Attila Doroszlai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62809/
> ---
> 
> (Updated Oct. 9, 2017, 12:43 p.m.)
> 
> 
> Review request for Ambari, Nishant Bangarwa, Slim Bouguerra, Sandor Magyari, 
> Swapan Shridhar, and Sebastian Toader.
> 
> 
> Bugs: AMBARI-22159
> https://issues.apache.org/jira/browse/AMBARI-22159
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Replace placeholders in `metastore_hostname`, 
> `druid.metadata.storage.connector.connectURI` and `druid.zk.service.host` 
> when creating cluster via blueprint.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
>  5a6e2cc0de7dc0c0f4499624e21c43bbc8d24b54 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
>  68d6349db0207ab6df44b4b0a341c77ecabc46ea 
> 
> 
> Diff: https://reviews.apache.org/r/62809/diff/2/
> 
> 
> Testing
> ---
> 
> Tested manually on local cluster.
> 
> Unit tests pass: `Tests run: 4900, Failures: 0, Errors: 0, Skipped: 36`
> 
> 
> Thanks,
> 
> Attila Doroszlai
> 
>

Re: Review Request 62809: AMBARI-22159. Replace hostgroup vars for Druid

[Robert Nettleton]

> On Oct. 7, 2017, noon, Oliver Szabo wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
> > Lines 2783 (patched)
> > 
> >
> > can these variables calculated at params.py in the stack code instead 
> > of putting to the BlueprintConfigurationProcessor? (using jinja2 variables 
> > at configurations - you can get the proper hosts from command.json input). 
> > just because the ambari server java code could be independent from druid. 
> > im not too big fan of this class, we should get rid of it sooner or later
> > 
> > but you can drop this issue if zou do not want to do it, it's just a 
> > reccomendation from me.
> 
> Attila Doroszlai wrote:
> I agree that this class stinks.  However, currently no `%HOSTGROUP::...%` 
> substitution occurs in Python code.  I guess it shouldn't be introduced at 
> this point before 2.6 release.
> 
> Oliver Szabo wrote:
> no need for %HOSTGROUP::.. substitution. It can be a jinja variable like 
> {{ metadata_connector_connect_uri }}.
> like for logsearch and ambari infra solr, there are some refrerences for 
> zookeeper but you cannot find that in the BlueprintConfigurationProcessor 
> class

Hi Oliver, 

I don't think the python substitution you mentioned will totally work.  

While the substitution might work on the agent side, I've seen cases where this 
type of substitution won't show up properly on the UI, meaning that the UI will 
show the jinja variable tokens in the config properties, which is not the 
correct behavior.

I agree that the BlueprintConfigurationProcessor should eventually be removed, 
but the problem is that the stack metadata doesn't hold enough information yet 
about topology data to allow this class to be refactored or removed just yet. 

Thanks, 
Bob


- Robert


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


On Oct. 9, 2017, 12:43 p.m., Attila Doroszlai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62809/
> ---
> 
> (Updated Oct. 9, 2017, 12:43 p.m.)
> 
> 
> Review request for Ambari, Nishant Bangarwa, Slim Bouguerra, Sandor Magyari, 
> Swapan Shridhar, and Sebastian Toader.
> 
> 
> Bugs: AMBARI-22159
> https://issues.apache.org/jira/browse/AMBARI-22159
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Replace placeholders in `metastore_hostname`, 
> `druid.metadata.storage.connector.connectURI` and `druid.zk.service.host` 
> when creating cluster via blueprint.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
>  5a6e2cc0de7dc0c0f4499624e21c43bbc8d24b54 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
>  68d6349db0207ab6df44b4b0a341c77ecabc46ea 
> 
> 
> Diff: https://reviews.apache.org/r/62809/diff/2/
> 
> 
> Testing
> ---
> 
> Tested manually on local cluster.
> 
> Unit tests pass: `Tests run: 4900, Failures: 0, Errors: 0, Skipped: 36`
> 
> 
> Thanks,
> 
> Attila Doroszlai
> 
>

Re: Review Request 62809: AMBARI-22159. Replace hostgroup vars for Druid

[Sebastian Toader]

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




ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
Line 1374 (original), 1420 (patched)


Can you do the same for `MultipleHostTopologyUpdater` and other updaters 
which currently process hostgroup plcaeholders?


- Sebastian Toader


On Oct. 9, 2017, 2:43 p.m., Attila Doroszlai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62809/
> ---
> 
> (Updated Oct. 9, 2017, 2:43 p.m.)
> 
> 
> Review request for Ambari, Nishant Bangarwa, Slim Bouguerra, Sandor Magyari, 
> Swapan Shridhar, and Sebastian Toader.
> 
> 
> Bugs: AMBARI-22159
> https://issues.apache.org/jira/browse/AMBARI-22159
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Replace placeholders in `metastore_hostname`, 
> `druid.metadata.storage.connector.connectURI` and `druid.zk.service.host` 
> when creating cluster via blueprint.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
>  5a6e2cc0de7dc0c0f4499624e21c43bbc8d24b54 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
>  68d6349db0207ab6df44b4b0a341c77ecabc46ea 
> 
> 
> Diff: https://reviews.apache.org/r/62809/diff/2/
> 
> 
> Testing
> ---
> 
> Tested manually on local cluster.
> 
> Unit tests pass: `Tests run: 4900, Failures: 0, Errors: 0, Skipped: 36`
> 
> 
> Thanks,
> 
> Attila Doroszlai
> 
>

Re: Review Request 62809: AMBARI-22159. Replace hostgroup vars for Druid

[Nishant Bangarwa]

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


Ship it!




Ship It!

- Nishant Bangarwa


On Oct. 9, 2017, 12:43 p.m., Attila Doroszlai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62809/
> ---
> 
> (Updated Oct. 9, 2017, 12:43 p.m.)
> 
> 
> Review request for Ambari, Nishant Bangarwa, Slim Bouguerra, Sandor Magyari, 
> Swapan Shridhar, and Sebastian Toader.
> 
> 
> Bugs: AMBARI-22159
> https://issues.apache.org/jira/browse/AMBARI-22159
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Replace placeholders in `metastore_hostname`, 
> `druid.metadata.storage.connector.connectURI` and `druid.zk.service.host` 
> when creating cluster via blueprint.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
>  5a6e2cc0de7dc0c0f4499624e21c43bbc8d24b54 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
>  68d6349db0207ab6df44b4b0a341c77ecabc46ea 
> 
> 
> Diff: https://reviews.apache.org/r/62809/diff/2/
> 
> 
> Testing
> ---
> 
> Tested manually on local cluster.
> 
> Unit tests pass: `Tests run: 4900, Failures: 0, Errors: 0, Skipped: 36`
> 
> 
> Thanks,
> 
> Attila Doroszlai
> 
>

Re: Review Request 62809: AMBARI-22159. Replace hostgroup vars for Druid

[Attila Doroszlai]

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

(Updated Oct. 9, 2017, 2:43 p.m.)


Review request for Ambari, Nishant Bangarwa, Slim Bouguerra, Sandor Magyari, 
Swapan Shridhar, and Sebastian Toader.


Changes
---

extracted property updater that only replaces `%HOSTGROUP::name%` placeholders


Bugs: AMBARI-22159
https://issues.apache.org/jira/browse/AMBARI-22159


Repository: ambari


Description
---

Replace placeholders in `metastore_hostname`, 
`druid.metadata.storage.connector.connectURI` and `druid.zk.service.host` when 
creating cluster via blueprint.


Diffs (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
 5a6e2cc0de7dc0c0f4499624e21c43bbc8d24b54 
  
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
 68d6349db0207ab6df44b4b0a341c77ecabc46ea 


Diff: https://reviews.apache.org/r/62809/diff/2/

Changes: https://reviews.apache.org/r/62809/diff/1-2/


Testing
---

Tested manually on local cluster.

Unit tests pass: `Tests run: 4900, Failures: 0, Errors: 0, Skipped: 36`


Thanks,

Attila Doroszlai

Re: Review Request 62809: AMBARI-22159. Replace hostgroup vars for Druid

[Sebastian Toader]

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


Ship it!




Ship It!

- Sebastian Toader


On Oct. 9, 2017, 9:16 a.m., Attila Doroszlai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62809/
> ---
> 
> (Updated Oct. 9, 2017, 9:16 a.m.)
> 
> 
> Review request for Ambari, Nishant Bangarwa, Slim Bouguerra, Sandor Magyari, 
> Swapan Shridhar, and Sebastian Toader.
> 
> 
> Bugs: AMBARI-22159
> https://issues.apache.org/jira/browse/AMBARI-22159
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Replace placeholders in `metastore_hostname`, 
> `druid.metadata.storage.connector.connectURI` and `druid.zk.service.host` 
> when creating cluster via blueprint.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
>  5a6e2cc0de7dc0c0f4499624e21c43bbc8d24b54 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
>  68d6349db0207ab6df44b4b0a341c77ecabc46ea 
> 
> 
> Diff: https://reviews.apache.org/r/62809/diff/1/
> 
> 
> Testing
> ---
> 
> Tested manually on local cluster.
> 
> Unit tests pass: `Tests run: 4900, Failures: 0, Errors: 0, Skipped: 36`
> 
> 
> Thanks,
> 
> Attila Doroszlai
> 
>

Re: Review Request 62809: AMBARI-22159. Replace hostgroup vars for Druid

[Attila Doroszlai]

> On Oct. 9, 2017, 11:41 a.m., Sebastian Toader wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
> > Lines 2782 (patched)
> > 
> >
> > Maybe we should introduce a property updater that handles only 
> > `%HOSTGROUP::name%` constructs.

Good idea.


> On Oct. 9, 2017, 11:41 a.m., Sebastian Toader wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
> > Lines 2784 (patched)
> > 
> >
> > Why property updater for `druid.zk.service.host` is not needed any more?

I don't understand the question.  The updater is being added by the patch, not 
removed.


- Attila


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


On Oct. 9, 2017, 9:16 a.m., Attila Doroszlai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62809/
> ---
> 
> (Updated Oct. 9, 2017, 9:16 a.m.)
> 
> 
> Review request for Ambari, Nishant Bangarwa, Slim Bouguerra, Sandor Magyari, 
> Swapan Shridhar, and Sebastian Toader.
> 
> 
> Bugs: AMBARI-22159
> https://issues.apache.org/jira/browse/AMBARI-22159
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Replace placeholders in `metastore_hostname`, 
> `druid.metadata.storage.connector.connectURI` and `druid.zk.service.host` 
> when creating cluster via blueprint.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
>  5a6e2cc0de7dc0c0f4499624e21c43bbc8d24b54 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
>  68d6349db0207ab6df44b4b0a341c77ecabc46ea 
> 
> 
> Diff: https://reviews.apache.org/r/62809/diff/1/
> 
> 
> Testing
> ---
> 
> Tested manually on local cluster.
> 
> Unit tests pass: `Tests run: 4900, Failures: 0, Errors: 0, Skipped: 36`
> 
> 
> Thanks,
> 
> Attila Doroszlai
> 
>

Re: Review Request 62809: AMBARI-22159. Replace hostgroup vars for Druid

[Sebastian Toader]

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




ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
Lines 2782 (patched)


Maybe we should introduce a property updater that handles only 
`%HOSTGROUP::name%` constructs.



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
Lines 2783 (patched)


Maybe we should introduce a new property updater for this property that 
handles only `%HOSTGROUP::name%` constructs.



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
Lines 2784 (patched)


Why property updater for `druid.zk.service.host` is not needed any more?


- Sebastian Toader


On Oct. 9, 2017, 9:16 a.m., Attila Doroszlai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62809/
> ---
> 
> (Updated Oct. 9, 2017, 9:16 a.m.)
> 
> 
> Review request for Ambari, Nishant Bangarwa, Slim Bouguerra, Sandor Magyari, 
> Swapan Shridhar, and Sebastian Toader.
> 
> 
> Bugs: AMBARI-22159
> https://issues.apache.org/jira/browse/AMBARI-22159
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Replace placeholders in `metastore_hostname`, 
> `druid.metadata.storage.connector.connectURI` and `druid.zk.service.host` 
> when creating cluster via blueprint.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
>  5a6e2cc0de7dc0c0f4499624e21c43bbc8d24b54 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
>  68d6349db0207ab6df44b4b0a341c77ecabc46ea 
> 
> 
> Diff: https://reviews.apache.org/r/62809/diff/1/
> 
> 
> Testing
> ---
> 
> Tested manually on local cluster.
> 
> Unit tests pass: `Tests run: 4900, Failures: 0, Errors: 0, Skipped: 36`
> 
> 
> Thanks,
> 
> Attila Doroszlai
> 
>

Re: Review Request 62809: AMBARI-22159. Replace hostgroup vars for Druid

[Oliver Szabo]

> On Oct. 7, 2017, noon, Oliver Szabo wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
> > Lines 2783 (patched)
> > 
> >
> > can these variables calculated at params.py in the stack code instead 
> > of putting to the BlueprintConfigurationProcessor? (using jinja2 variables 
> > at configurations - you can get the proper hosts from command.json input). 
> > just because the ambari server java code could be independent from druid. 
> > im not too big fan of this class, we should get rid of it sooner or later
> > 
> > but you can drop this issue if zou do not want to do it, it's just a 
> > reccomendation from me.
> 
> Attila Doroszlai wrote:
> I agree that this class stinks.  However, currently no `%HOSTGROUP::...%` 
> substitution occurs in Python code.  I guess it shouldn't be introduced at 
> this point before 2.6 release.

no need for %HOSTGROUP::.. substitution. It can be a jinja variable like {{ 
metadata_connector_connect_uri }}.
like for logsearch and ambari infra solr, there are some refrerences for 
zookeeper but you cannot find that in the BlueprintConfigurationProcessor class


- Oliver


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


On Oct. 6, 2017, 3:35 p.m., Attila Doroszlai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62809/
> ---
> 
> (Updated Oct. 6, 2017, 3:35 p.m.)
> 
> 
> Review request for Ambari, Nishant Bangarwa, Slim Bouguerra, and Swapan 
> Shridhar.
> 
> 
> Bugs: AMBARI-22159
> https://issues.apache.org/jira/browse/AMBARI-22159
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Replace placeholders in `metastore_hostname`, 
> `druid.metadata.storage.connector.connectURI` and `druid.zk.service.host` 
> when creating cluster via blueprint.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
>  5a6e2cc0de7dc0c0f4499624e21c43bbc8d24b54 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
>  68d6349db0207ab6df44b4b0a341c77ecabc46ea 
> 
> 
> Diff: https://reviews.apache.org/r/62809/diff/1/
> 
> 
> Testing
> ---
> 
> Tested manually on local cluster.
> 
> Unit tests pass: `Tests run: 4900, Failures: 0, Errors: 0, Skipped: 36`
> 
> 
> Thanks,
> 
> Attila Doroszlai
> 
>

Re: Review Request 62809: AMBARI-22159. Replace hostgroup vars for Druid

[Attila Doroszlai]

> On Oct. 7, 2017, 2 p.m., Oliver Szabo wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
> > Lines 2783 (patched)
> > 
> >
> > can these variables calculated at params.py in the stack code instead 
> > of putting to the BlueprintConfigurationProcessor? (using jinja2 variables 
> > at configurations - you can get the proper hosts from command.json input). 
> > just because the ambari server java code could be independent from druid. 
> > im not too big fan of this class, we should get rid of it sooner or later
> > 
> > but you can drop this issue if zou do not want to do it, it's just a 
> > reccomendation from me.

I agree that this class stinks.  However, currently no `%HOSTGROUP::...%` 
substitution occurs in Python code.  I guess it shouldn't be introduced at this 
point before 2.6 release.


- Attila


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


On Oct. 6, 2017, 5:35 p.m., Attila Doroszlai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62809/
> ---
> 
> (Updated Oct. 6, 2017, 5:35 p.m.)
> 
> 
> Review request for Ambari, Nishant Bangarwa, Slim Bouguerra, and Swapan 
> Shridhar.
> 
> 
> Bugs: AMBARI-22159
> https://issues.apache.org/jira/browse/AMBARI-22159
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Replace placeholders in `metastore_hostname`, 
> `druid.metadata.storage.connector.connectURI` and `druid.zk.service.host` 
> when creating cluster via blueprint.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
>  5a6e2cc0de7dc0c0f4499624e21c43bbc8d24b54 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
>  68d6349db0207ab6df44b4b0a341c77ecabc46ea 
> 
> 
> Diff: https://reviews.apache.org/r/62809/diff/1/
> 
> 
> Testing
> ---
> 
> Tested manually on local cluster.
> 
> Unit tests pass: `Tests run: 4900, Failures: 0, Errors: 0, Skipped: 36`
> 
> 
> Thanks,
> 
> Attila Doroszlai
> 
>

Re: Review Request 62809: AMBARI-22159. Replace hostgroup vars for Druid

[Oliver Szabo]

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




ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
Lines 2783 (patched)


can these variables calculated at params.py in the stack code instead of 
putting to the BlueprintConfigurationProcessor? (using jinja2 variables at 
configurations - you can get the proper hosts from command.json input). just 
because the ambari server java code could be independent from druid. im not too 
big fan of this class, we should get rid of it sooner or later

but you can drop this issue if zou do not want to do it, it's just a 
reccomendation from me.


- Oliver Szabo


On Oct. 6, 2017, 3:35 p.m., Attila Doroszlai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62809/
> ---
> 
> (Updated Oct. 6, 2017, 3:35 p.m.)
> 
> 
> Review request for Ambari, Nishant Bangarwa, Slim Bouguerra, and Swapan 
> Shridhar.
> 
> 
> Bugs: AMBARI-22159
> https://issues.apache.org/jira/browse/AMBARI-22159
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Replace placeholders in `metastore_hostname`, 
> `druid.metadata.storage.connector.connectURI` and `druid.zk.service.host` 
> when creating cluster via blueprint.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
>  5a6e2cc0de7dc0c0f4499624e21c43bbc8d24b54 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
>  68d6349db0207ab6df44b4b0a341c77ecabc46ea 
> 
> 
> Diff: https://reviews.apache.org/r/62809/diff/1/
> 
> 
> Testing
> ---
> 
> Tested manually on local cluster.
> 
> Unit tests pass: `Tests run: 4900, Failures: 0, Errors: 0, Skipped: 36`
> 
> 
> Thanks,
> 
> Attila Doroszlai
> 
>

Re: Review Request 62809: AMBARI-22159. Replace hostgroup vars for Druid

[Attila Doroszlai]

> On Oct. 6, 2017, 9:47 p.m., Nishant Bangarwa wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
> > Lines 2782 (patched)
> > 
> >
> > this seems odd, metadata store is not always installed on the same host 
> > as coordinator. 
> > Fwiw, metadata store for druid is not installed/managed by ambari, user 
> > needs to set it up and provide valid hostname and uri.

For explicit `%HOSTGROUP::some_host_group%` the hostname from the given host 
group is substituted, component is ignored.
Component ("DRUID_COORDINATOR") is relevant only if property value contains 
"localhost".


- Attila


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


On Oct. 6, 2017, 5:35 p.m., Attila Doroszlai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62809/
> ---
> 
> (Updated Oct. 6, 2017, 5:35 p.m.)
> 
> 
> Review request for Ambari, Nishant Bangarwa, Slim Bouguerra, and Swapan 
> Shridhar.
> 
> 
> Bugs: AMBARI-22159
> https://issues.apache.org/jira/browse/AMBARI-22159
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Replace placeholders in `metastore_hostname`, 
> `druid.metadata.storage.connector.connectURI` and `druid.zk.service.host` 
> when creating cluster via blueprint.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
>  5a6e2cc0de7dc0c0f4499624e21c43bbc8d24b54 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
>  68d6349db0207ab6df44b4b0a341c77ecabc46ea 
> 
> 
> Diff: https://reviews.apache.org/r/62809/diff/1/
> 
> 
> Testing
> ---
> 
> Tested manually on local cluster.
> 
> Unit tests pass: `Tests run: 4900, Failures: 0, Errors: 0, Skipped: 36`
> 
> 
> Thanks,
> 
> Attila Doroszlai
> 
>

Re: Review Request 62809: AMBARI-22159. Replace hostgroup vars for Druid

[Nishant Bangarwa]

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




ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
Lines 2782 (patched)


this seems odd, metadata store is not always installed on the same host as 
coordinator. 
Fwiw, metadata store for druid is not installed/managed by ambari, user 
needs to set it up and provide valid hostname and uri.


- Nishant Bangarwa


On Oct. 6, 2017, 3:35 p.m., Attila Doroszlai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62809/
> ---
> 
> (Updated Oct. 6, 2017, 3:35 p.m.)
> 
> 
> Review request for Ambari, Nishant Bangarwa, Slim Bouguerra, and Swapan 
> Shridhar.
> 
> 
> Bugs: AMBARI-22159
> https://issues.apache.org/jira/browse/AMBARI-22159
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Replace placeholders in `metastore_hostname`, 
> `druid.metadata.storage.connector.connectURI` and `druid.zk.service.host` 
> when creating cluster via blueprint.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
>  5a6e2cc0de7dc0c0f4499624e21c43bbc8d24b54 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
>  68d6349db0207ab6df44b4b0a341c77ecabc46ea 
> 
> 
> Diff: https://reviews.apache.org/r/62809/diff/1/
> 
> 
> Testing
> ---
> 
> Tested manually on local cluster.
> 
> Unit tests pass: `Tests run: 4900, Failures: 0, Errors: 0, Skipped: 36`
> 
> 
> Thanks,
> 
> Attila Doroszlai
> 
>

Re: Review Request 62809: AMBARI-22159. Replace hostgroup vars for Druid

[Attila Doroszlai]

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

(Updated Oct. 6, 2017, 5:35 p.m.)


Review request for Ambari, Nishant Bangarwa, Slim Bouguerra, and Swapan 
Shridhar.


Bugs: AMBARI-22159
https://issues.apache.org/jira/browse/AMBARI-22159


Repository: ambari


Description
---

Replace placeholders in `metastore_hostname`, 
`druid.metadata.storage.connector.connectURI` and `druid.zk.service.host` when 
creating cluster via blueprint.


Diffs
-

  
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
 5a6e2cc0de7dc0c0f4499624e21c43bbc8d24b54 
  
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
 68d6349db0207ab6df44b4b0a341c77ecabc46ea 


Diff: https://reviews.apache.org/r/62809/diff/1/


Testing (updated)
---

Tested manually on local cluster.

Unit tests pass: `Tests run: 4900, Failures: 0, Errors: 0, Skipped: 36`


Thanks,

Attila Doroszlai