[GitHub] cloudstack pull request: CLOUDSTACK-8666: Put host in Alert state ...

2015-07-27 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/621#issuecomment-12513
  
Hi @koushik-das 

Perhaps refactoring the code in a way that its design is improved could 
make easy to add tests for it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-8666: Put host in Alert state ...

2015-07-27 Thread koushik-das
Github user koushik-das commented on the pull request:

https://github.com/apache/cloudstack/pull/621#issuecomment-125156829
  
@wilderrodrigues Agree that refactoring the code might help with the tests. 
But that will be a bigger effort.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-8666: Put host in Alert state ...

2015-07-27 Thread koushik-das
Github user koushik-das commented on the pull request:

https://github.com/apache/cloudstack/pull/621#issuecomment-125152445
  
@DaanHoogland Writing an unit test for changes in AgentManagerImpl.java is 
hard due to the way the method gets invoked. Let me know if you have any 
suggestion.
Fot the HighAvailabilityManagerImpl.java changes I have written a unit 
test, will send out the PR for that.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-8666: Put host in Alert state ...

2015-07-24 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/cloudstack/pull/621


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-8666: Put host in Alert state ...

2015-07-24 Thread koushik-das
Github user koushik-das commented on the pull request:

https://github.com/apache/cloudstack/pull/621#issuecomment-124357139
  
Merging it as 2 LGTMs


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-8666: Put host in Alert state ...

2015-07-24 Thread DaanHoogland
Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/621#issuecomment-124584731
  
wasn't this needing (unit)-tests?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-8666: Put host in Alert state ...

2015-07-23 Thread wido
Github user wido commented on the pull request:

https://github.com/apache/cloudstack/pull/621#issuecomment-124243179
  
LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-8666: Put host in Alert state ...

2015-07-23 Thread kishankavala
Github user kishankavala commented on the pull request:

https://github.com/apache/cloudstack/pull/621#issuecomment-124323020
  
LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-8666: Put host in Alert state ...

2015-07-23 Thread koushik-das
GitHub user koushik-das opened a pull request:

https://github.com/apache/cloudstack/pull/621

CLOUDSTACK-8666: Put host in Alert state only after alert.wait timeout

Instead of putting the host to Alert state immediately, the investigators 
should be allowed to run for some time based on alert.wait global config.
At the end of this interval if the host state still cannot be determined 
then put the host in Alert. Also updated some of the log messages.

Refer to the bug for the detailed description.

Since these scenarios are difficult to simulate, haven't written any tests. 
If anyone has suggestions on some tests please let me know.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/koushik-das/cloudstack CLOUDSTACK-8666

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/621.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #621


commit 2cd8e6726540186b9ccec41beebe21af3a6d08b6
Author: Koushik Das kous...@apache.org
Date:   2015-07-23T12:27:51Z

CLOUDSTACK-8666: Put host in Alert state only after alert.wait timeout
Instead of putting the host to Alert state immediately, the investigators 
should be allowed to run for some time based on alert.wait global config.
At the end of this interval if the host state still cannot be determined 
then put the host in Alert. Also updated some of the log messages.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---