Re: Review Request 62756: Refactor AMS logic in stack advisors to service advisors

2017-10-05 Thread Jayush Luniya 

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


Ship it!




Ship It!

- Jayush Luniya


On Oct. 5, 2017, 2:22 p.m., Vitalyi Brodetskyi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62756/
> ---
> 
> (Updated Oct. 5, 2017, 2:22 p.m.)
> 
> 
> Review request for Ambari, Aravindan Vijayan, Jayush Luniya, Madhuvanthi 
> Radhakrishnan, and Sid Wagle.
> 
> 
> Bugs: AMBARI-22124
> https://issues.apache.org/jira/browse/AMBARI-22124
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> We need to move AMS specific logic in the stack advisors to service advisor.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/service_advisor.py
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/common-services/ATLAS/0.7.0.3.0/service_advisor.py
>  a2e31cc 
>   ambari-server/src/main/resources/stacks/HDP/2.0.6/services/stack_advisor.py 
> 5307176 
>   ambari-server/src/main/resources/stacks/HDP/2.2/services/stack_advisor.py 
> 2dc1738 
>   
> ambari-server/src/test/python/common-services/AMBARI_METRICS/test_service_advisor.py
>  PRE-CREATION 
>   ambari-server/src/test/python/stacks/2.0.6/common/test_stack_advisor.py 
> 65b23b0 
>   ambari-server/src/test/python/stacks/2.2/common/test_stack_advisor.py 
> d6b572e 
> 
> 
> Diff: https://reviews.apache.org/r/62756/diff/4/
> 
> 
> Testing
> ---
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Vitalyi Brodetskyi
> 
>

Re: Review Request 62756: Refactor AMS logic in stack advisors to service advisors

2017-10-05 Thread Vitalyi Brodetskyi 

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

(Updated Жов. 5, 2017, 2:22 після полудня)


Review request for Ambari, Aravindan Vijayan, Jayush Luniya, Madhuvanthi 
Radhakrishnan, and Sid Wagle.


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


Repository: ambari


Description
---

We need to move AMS specific logic in the stack advisors to service advisor.


Diffs (updated)
-

  
ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/service_advisor.py
 PRE-CREATION 
  
ambari-server/src/main/resources/common-services/ATLAS/0.7.0.3.0/service_advisor.py
 a2e31cc 
  ambari-server/src/main/resources/stacks/HDP/2.0.6/services/stack_advisor.py 
5307176 
  ambari-server/src/main/resources/stacks/HDP/2.2/services/stack_advisor.py 
2dc1738 
  
ambari-server/src/test/python/common-services/AMBARI_METRICS/test_service_advisor.py
 PRE-CREATION 
  ambari-server/src/test/python/stacks/2.0.6/common/test_stack_advisor.py 
65b23b0 
  ambari-server/src/test/python/stacks/2.2/common/test_stack_advisor.py d6b572e 


Diff: https://reviews.apache.org/r/62756/diff/4/

Changes: https://reviews.apache.org/r/62756/diff/3-4/


Testing
---

mvn clean test


Thanks,

Vitalyi Brodetskyi

Re: Review Request 62756: Refactor AMS logic in stack advisors to service advisors

2017-10-04 Thread Jayush Luniya 

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




ambari-server/src/test/python/stacks/2.0.6/common/test_stack_advisor.py
Line 2318 (original)


Can't we move the test code to 
ambari-server/src/test/python/common-services/AMBARI_METRICS/test_service_advisor.py?

See 
https://github.com/apache/ambari/blob/trunk/ambari-server/src/test/python/common-services/LOGSEARCH/test_service_advisor.py


- Jayush Luniya


On Oct. 4, 2017, 9:03 p.m., Vitalyi Brodetskyi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62756/
> ---
> 
> (Updated Oct. 4, 2017, 9:03 p.m.)
> 
> 
> Review request for Ambari, Aravindan Vijayan, Jayush Luniya, Madhuvanthi 
> Radhakrishnan, and Sid Wagle.
> 
> 
> Bugs: AMBARI-22124
> https://issues.apache.org/jira/browse/AMBARI-22124
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> We need to move AMS specific logic in the stack advisors to service advisor.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/service_advisor.py
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/common-services/ATLAS/0.7.0.3.0/service_advisor.py
>  a2e31cc 
>   ambari-server/src/main/resources/stacks/HDP/2.0.6/services/stack_advisor.py 
> 5307176 
>   ambari-server/src/main/resources/stacks/HDP/2.2/services/stack_advisor.py 
> 2dc1738 
>   ambari-server/src/test/python/stacks/2.0.6/common/test_stack_advisor.py 
> 65b23b0 
>   ambari-server/src/test/python/stacks/2.2/common/test_stack_advisor.py 
> d6b572e 
> 
> 
> Diff: https://reviews.apache.org/r/62756/diff/3/
> 
> 
> Testing
> ---
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Vitalyi Brodetskyi
> 
>

Re: Review Request 62756: Refactor AMS logic in stack advisors to service advisors

2017-10-04 Thread Vitalyi Brodetskyi 

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

(Updated Жов. 4, 2017, 9:03 після полудня)


Review request for Ambari, Aravindan Vijayan, Jayush Luniya, Madhuvanthi 
Radhakrishnan, and Sid Wagle.


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


Repository: ambari


Description
---

We need to move AMS specific logic in the stack advisors to service advisor.


Diffs (updated)
-

  
ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/service_advisor.py
 PRE-CREATION 
  
ambari-server/src/main/resources/common-services/ATLAS/0.7.0.3.0/service_advisor.py
 a2e31cc 
  ambari-server/src/main/resources/stacks/HDP/2.0.6/services/stack_advisor.py 
5307176 
  ambari-server/src/main/resources/stacks/HDP/2.2/services/stack_advisor.py 
2dc1738 
  ambari-server/src/test/python/stacks/2.0.6/common/test_stack_advisor.py 
65b23b0 
  ambari-server/src/test/python/stacks/2.2/common/test_stack_advisor.py d6b572e 


Diff: https://reviews.apache.org/r/62756/diff/3/

Changes: https://reviews.apache.org/r/62756/diff/2-3/


Testing
---

mvn clean test


Thanks,

Vitalyi Brodetskyi

Re: Review Request 62756: Refactor AMS logic in stack advisors to service advisors

2017-10-04 Thread Jayush Luniya 


> On Oct. 4, 2017, 12:54 a.m., Jayush Luniya wrote:
> > ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/service_advisor.py
> > Lines 21 (patched)
> > 
> >
> > Can we also remove the duplicate code in HDP stack advisors?
> 
> Vitalyi Brodetskyi wrote:
> Which code i should remove? If you are talking about all AMS methods 
> (recommendations and validations), i don't think we can remove them. As of 
> now, only HDP 3.0 work with service advisors, all other stacks work with 
> stack advisor.

No support for service advisor was add a while back. We already have LOGSEARCH, 
AMBARI_INFRA, NIFI that define service advisors. 

See 
https://github.com/apache/ambari/blob/trunk/ambari-server/src/main/resources/common-services/LOGSEARCH/0.5.0/service_advisor.py

The default stack advisor has installation hooks to call individual service 
advisors
https://github.com/apache/ambari/blob/trunk/ambari-server/src/main/resources/stacks/stack_advisor.py#L765

Can you try and see?


- Jayush


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


On Oct. 4, 2017, 8:26 a.m., Vitalyi Brodetskyi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62756/
> ---
> 
> (Updated Oct. 4, 2017, 8:26 a.m.)
> 
> 
> Review request for Ambari, Aravindan Vijayan, Jayush Luniya, Madhuvanthi 
> Radhakrishnan, and Sid Wagle.
> 
> 
> Bugs: AMBARI-22124
> https://issues.apache.org/jira/browse/AMBARI-22124
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> We need to move AMS specific logic in the stack advisors to service advisor.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/service_advisor.py
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/common-services/ATLAS/0.7.0.3.0/service_advisor.py
>  a2e31cc 
> 
> 
> Diff: https://reviews.apache.org/r/62756/diff/2/
> 
> 
> Testing
> ---
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Vitalyi Brodetskyi
> 
>

Re: Review Request 62756: Refactor AMS logic in stack advisors to service advisors

2017-10-04 Thread Vitalyi Brodetskyi 


> On Жов. 4, 2017, 12:54 до полудня, Jayush Luniya wrote:
> > ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/service_advisor.py
> > Lines 21 (patched)
> > 
> >
> > Can we also remove the duplicate code in HDP stack advisors?

Which code i should remove? If you are talking about all AMS methods 
(recommendations and validations), i don't think we can remove them. As of now, 
only HDP 3.0 work with service advisors, all other stacks work with stack 
advisor.


- Vitalyi


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


On Жов. 4, 2017, 8:26 до полудня, Vitalyi Brodetskyi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62756/
> ---
> 
> (Updated Жов. 4, 2017, 8:26 до полудня)
> 
> 
> Review request for Ambari, Aravindan Vijayan, Jayush Luniya, Madhuvanthi 
> Radhakrishnan, and Sid Wagle.
> 
> 
> Bugs: AMBARI-22124
> https://issues.apache.org/jira/browse/AMBARI-22124
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> We need to move AMS specific logic in the stack advisors to service advisor.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/service_advisor.py
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/common-services/ATLAS/0.7.0.3.0/service_advisor.py
>  a2e31cc 
> 
> 
> Diff: https://reviews.apache.org/r/62756/diff/2/
> 
> 
> Testing
> ---
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Vitalyi Brodetskyi
> 
>

Re: Review Request 62756: Refactor AMS logic in stack advisors to service advisors

2017-10-04 Thread Vitalyi Brodetskyi 


> On Жов. 4, 2017, 12:49 до полудня, Jayush Luniya wrote:
> > ambari-server/src/main/resources/common-services/ATLAS/0.7.0.3.0/service_advisor.py
> > Line 77 (original), 77 (patched)
> > 
> >
> > Is this change to ATLAS service advisor supposed to be part of the 
> > patch?

Just found this little part of missed code during work on service advisor for 
AMS. Decided to add it here because change is trivial.


- Vitalyi


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


On Жов. 4, 2017, 8:26 до полудня, Vitalyi Brodetskyi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62756/
> ---
> 
> (Updated Жов. 4, 2017, 8:26 до полудня)
> 
> 
> Review request for Ambari, Aravindan Vijayan, Jayush Luniya, Madhuvanthi 
> Radhakrishnan, and Sid Wagle.
> 
> 
> Bugs: AMBARI-22124
> https://issues.apache.org/jira/browse/AMBARI-22124
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> We need to move AMS specific logic in the stack advisors to service advisor.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/service_advisor.py
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/common-services/ATLAS/0.7.0.3.0/service_advisor.py
>  a2e31cc 
> 
> 
> Diff: https://reviews.apache.org/r/62756/diff/2/
> 
> 
> Testing
> ---
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Vitalyi Brodetskyi
> 
>

Re: Review Request 62756: Refactor AMS logic in stack advisors to service advisors

2017-10-04 Thread Vitalyi Brodetskyi 

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

(Updated Жов. 4, 2017, 8:26 до полудня)


Review request for Ambari, Aravindan Vijayan, Jayush Luniya, Madhuvanthi 
Radhakrishnan, and Sid Wagle.


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


Repository: ambari


Description
---

We need to move AMS specific logic in the stack advisors to service advisor.


Diffs (updated)
-

  
ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/service_advisor.py
 PRE-CREATION 
  
ambari-server/src/main/resources/common-services/ATLAS/0.7.0.3.0/service_advisor.py
 a2e31cc 


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

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


Testing
---

mvn clean test


Thanks,

Vitalyi Brodetskyi

Re: Review Request 62756: Refactor AMS logic in stack advisors to service advisors

2017-10-03 Thread Jayush Luniya 

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




ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/service_advisor.py
Lines 21 (patched)


Can we also remove the duplicate code in HDP stack advisors?



ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/service_advisor.py
Lines 466 (patched)


Fix Typo "AMS Validator" instead of "Storm Validator"


- Jayush Luniya


On Oct. 3, 2017, 7:48 p.m., Vitalyi Brodetskyi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62756/
> ---
> 
> (Updated Oct. 3, 2017, 7:48 p.m.)
> 
> 
> Review request for Ambari, Aravindan Vijayan, Jayush Luniya, Madhuvanthi 
> Radhakrishnan, and Sid Wagle.
> 
> 
> Bugs: AMBARI-22124
> https://issues.apache.org/jira/browse/AMBARI-22124
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> We need to move AMS specific logic in the stack advisors to service advisor.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/service_advisor.py
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/common-services/ATLAS/0.7.0.3.0/service_advisor.py
>  a2e31cc 
> 
> 
> Diff: https://reviews.apache.org/r/62756/diff/1/
> 
> 
> Testing
> ---
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Vitalyi Brodetskyi
> 
>

Re: Review Request 62756: Refactor AMS logic in stack advisors to service advisors

2017-10-03 Thread Jayush Luniya 

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




ambari-server/src/main/resources/common-services/ATLAS/0.7.0.3.0/service_advisor.py
Line 77 (original), 77 (patched)


Is this change to ATLAS service advisor supposed to be part of the patch?


- Jayush Luniya


On Oct. 3, 2017, 7:48 p.m., Vitalyi Brodetskyi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62756/
> ---
> 
> (Updated Oct. 3, 2017, 7:48 p.m.)
> 
> 
> Review request for Ambari, Aravindan Vijayan, Jayush Luniya, Madhuvanthi 
> Radhakrishnan, and Sid Wagle.
> 
> 
> Bugs: AMBARI-22124
> https://issues.apache.org/jira/browse/AMBARI-22124
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> We need to move AMS specific logic in the stack advisors to service advisor.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/service_advisor.py
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/common-services/ATLAS/0.7.0.3.0/service_advisor.py
>  a2e31cc 
> 
> 
> Diff: https://reviews.apache.org/r/62756/diff/1/
> 
> 
> Testing
> ---
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Vitalyi Brodetskyi
> 
>

Re: Review Request 62756: Refactor AMS logic in stack advisors to service advisors

2017-10-03 Thread Aravindan Vijayan 

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


Ship it!




Ship It!

- Aravindan Vijayan


On Oct. 3, 2017, 7:48 p.m., Vitalyi Brodetskyi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62756/
> ---
> 
> (Updated Oct. 3, 2017, 7:48 p.m.)
> 
> 
> Review request for Ambari, Aravindan Vijayan, Jayush Luniya, Madhuvanthi 
> Radhakrishnan, and Sid Wagle.
> 
> 
> Bugs: AMBARI-22124
> https://issues.apache.org/jira/browse/AMBARI-22124
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> We need to move AMS specific logic in the stack advisors to service advisor.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/service_advisor.py
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/common-services/ATLAS/0.7.0.3.0/service_advisor.py
>  a2e31cc 
> 
> 
> Diff: https://reviews.apache.org/r/62756/diff/1/
> 
> 
> Testing
> ---
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Vitalyi Brodetskyi
> 
>

Review Request 62756: Refactor AMS logic in stack advisors to service advisors

2017-10-03 Thread Vitalyi Brodetskyi 

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

Review request for Ambari, Jayush Luniya and Madhuvanthi Radhakrishnan.


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


Repository: ambari


Description
---

We need to move AMS specific logic in the stack advisors to service advisor.


Diffs
-

  
ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/service_advisor.py
 PRE-CREATION 
  
ambari-server/src/main/resources/common-services/ATLAS/0.7.0.3.0/service_advisor.py
 a2e31cc 


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


Testing
---

mvn clean test


Thanks,

Vitalyi Brodetskyi