Re: Review Request 63618: Multiple issue with handling unexcepted situations

2017-11-07 Thread Andrew Onischuk 


> On Nov. 7, 2017, 3:14 p.m., Attila Doroszlai wrote:
> > ambari-agent/src/main/python/ambari_agent/ClusterCache.py
> > Lines 99-100 (original), 103-106 (patched)
> > 
> >
> > `persist_cache()` will write possibly obsolete `self.hash`, not the new 
> > one.  Should pass `cache_hash` as argument now that the order is reversed.

Nice find.


- Andrew


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


On Nov. 7, 2017, 1:48 p.m., Andrew Onischuk wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63618/
> ---
> 
> (Updated Nov. 7, 2017, 1:48 p.m.)
> 
> 
> Review request for Ambari, Attila Doroszlai, Myroslav Papirkovskyy, and Sid 
> Wagle.
> 
> 
> Bugs: AMBARI-22375
> https://issues.apache.org/jira/browse/AMBARI-22375
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> * If on_update_cache fails cache should be discard
>   * loggers should log messages during initialization of classes
>   * hash of cache should only updated if saving/processing it was sucessful
> 
> 
> Diffs
> -
> 
>   ambari-agent/src/main/python/ambari_agent/ClusterCache.py b924420f8b 
>   ambari-agent/src/main/python/ambari_agent/InitializerModule.py 2c8021844f 
>   ambari-agent/src/main/python/ambari_agent/main.py 3948290519 
>   ambari-agent/src/test/python/ambari_agent/TestAgentStompResponses.py 
> 2154fca8d3 
> 
> 
> Diff: https://reviews.apache.org/r/63618/diff/1/
> 
> 
> Testing
> ---
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Andrew Onischuk
> 
>

Re: Review Request 63618: Multiple issue with handling unexcepted situations

2017-11-07 Thread Attila Doroszlai 

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


Fix it, then Ship it!





ambari-agent/src/main/python/ambari_agent/ClusterCache.py
Lines 78 (patched)


The cache still may end up in a partially updated state, since this does 
not clear it, just a local variable.



ambari-agent/src/main/python/ambari_agent/ClusterCache.py
Lines 99-100 (original), 103-106 (patched)


`persist_cache()` will write possibly obsolete `self.hash`, not the new 
one.  Should pass `cache_hash` as argument now that the order is reversed.


- Attila Doroszlai


On Nov. 7, 2017, 2:48 p.m., Andrew Onischuk wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63618/
> ---
> 
> (Updated Nov. 7, 2017, 2:48 p.m.)
> 
> 
> Review request for Ambari, Attila Doroszlai, Myroslav Papirkovskyy, and Sid 
> Wagle.
> 
> 
> Bugs: AMBARI-22375
> https://issues.apache.org/jira/browse/AMBARI-22375
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> * If on_update_cache fails cache should be discard
>   * loggers should log messages during initialization of classes
>   * hash of cache should only updated if saving/processing it was sucessful
> 
> 
> Diffs
> -
> 
>   ambari-agent/src/main/python/ambari_agent/ClusterCache.py b924420f8b 
>   ambari-agent/src/main/python/ambari_agent/InitializerModule.py 2c8021844f 
>   ambari-agent/src/main/python/ambari_agent/main.py 3948290519 
>   ambari-agent/src/test/python/ambari_agent/TestAgentStompResponses.py 
> 2154fca8d3 
> 
> 
> Diff: https://reviews.apache.org/r/63618/diff/1/
> 
> 
> Testing
> ---
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Andrew Onischuk
> 
>

Review Request 63618: Multiple issue with handling unexcepted situations

2017-11-07 Thread Andrew Onischuk 

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

Review request for Ambari, Attila Doroszlai, Myroslav Papirkovskyy, and Sid 
Wagle.


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


Repository: ambari


Description
---

* If on_update_cache fails cache should be discard
  * loggers should log messages during initialization of classes
  * hash of cache should only updated if saving/processing it was sucessful


Diffs
-

  ambari-agent/src/main/python/ambari_agent/ClusterCache.py b924420f8b 
  ambari-agent/src/main/python/ambari_agent/InitializerModule.py 2c8021844f 
  ambari-agent/src/main/python/ambari_agent/main.py 3948290519 
  ambari-agent/src/test/python/ambari_agent/TestAgentStompResponses.py 
2154fca8d3 


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


Testing
---

mvn clean test


Thanks,

Andrew Onischuk