You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Santhosh Edukulla <sa...@citrix.com> on 2014/05/05 08:49:36 UTC
Review Request 21070: Fixed few exception cases.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21070/
-----------------------------------------------------------
Review request for cloudstack and sanjeev n.
Repository: cloudstack-git
Description
-------
Fixed few exception cases. When some request fails, it throws up the stack, but the message str..does not contains a given attribute is little misleading. So, raising exceptions for all cases.
Added testsuite name for userprovided\default log folder path
Fixed pep8 changes.
Diffs
-----
tools/marvin/marvin/cloudstackConnection.py caa8609
tools/marvin/marvin/cloudstackTestClient.py d3a6b94
tools/marvin/marvin/configGenerator.py 4f03fd0
tools/marvin/marvin/marvinInit.py de580ce
tools/marvin/marvin/marvinLog.py 65687aa
tools/marvin/marvin/marvinPlugin.py c817cd6
Diff: https://reviews.apache.org/r/21070/diff/
Testing
-------
Thanks,
Santhosh Edukulla
Re: Review Request 21070: Fixed few exception cases.
Posted by sanjeev n <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21070/#review42142
-----------------------------------------------------------
Please rebase with master since patch apply failed on master branch.
- sanjeev n
On May 5, 2014, 6:49 a.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21070/
> -----------------------------------------------------------
>
> (Updated May 5, 2014, 6:49 a.m.)
>
>
> Review request for cloudstack and sanjeev n.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed few exception cases. When some request fails, it throws up the stack, but the message str..does not contains a given attribute is little misleading. So, raising exceptions for all cases.
> Added testsuite name for userprovided\default log folder path
> Fixed pep8 changes.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/cloudstackConnection.py caa8609
> tools/marvin/marvin/cloudstackTestClient.py d3a6b94
> tools/marvin/marvin/configGenerator.py 4f03fd0
> tools/marvin/marvin/marvinInit.py de580ce
> tools/marvin/marvin/marvinLog.py 65687aa
> tools/marvin/marvin/marvinPlugin.py c817cd6
>
> Diff: https://reviews.apache.org/r/21070/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 21070: Fixed few exception cases.
Posted by Santhosh Edukulla <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21070/#review42491
-----------------------------------------------------------
Ship it!
Ship It!
- Santhosh Edukulla
On May 5, 2014, 6:49 a.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21070/
> -----------------------------------------------------------
>
> (Updated May 5, 2014, 6:49 a.m.)
>
>
> Review request for cloudstack and sanjeev n.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed few exception cases. When some request fails, it throws up the stack, but the message str..does not contains a given attribute is little misleading. So, raising exceptions for all cases.
> Added testsuite name for userprovided\default log folder path
> Fixed pep8 changes.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/cloudstackConnection.py caa8609
> tools/marvin/marvin/cloudstackTestClient.py d3a6b94
> tools/marvin/marvin/configGenerator.py 4f03fd0
> tools/marvin/marvin/marvinInit.py de580ce
> tools/marvin/marvin/marvinLog.py 65687aa
> tools/marvin/marvin/marvinPlugin.py c817cd6
>
> Diff: https://reviews.apache.org/r/21070/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 21070: Fixed few exception cases.
Posted by sanjeev n <sa...@citrix.com>.
> On May 5, 2014, 4:06 p.m., sanjeev n wrote:
> > Ship It!
Applied patch to 4.4-automation branch
commit ef286da8ccc005fee58c4be5d508c5d7ddfc2a6c
Author: sanjeevneelarapu <sa...@citrix.com>
Date: Tue May 6 02:57:08 2014 +0530
Fixed few exception cases
- sanjeev
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21070/#review42157
-----------------------------------------------------------
On May 5, 2014, 6:49 a.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21070/
> -----------------------------------------------------------
>
> (Updated May 5, 2014, 6:49 a.m.)
>
>
> Review request for cloudstack and sanjeev n.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed few exception cases. When some request fails, it throws up the stack, but the message str..does not contains a given attribute is little misleading. So, raising exceptions for all cases.
> Added testsuite name for userprovided\default log folder path
> Fixed pep8 changes.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/cloudstackConnection.py caa8609
> tools/marvin/marvin/cloudstackTestClient.py d3a6b94
> tools/marvin/marvin/configGenerator.py 4f03fd0
> tools/marvin/marvin/marvinInit.py de580ce
> tools/marvin/marvin/marvinLog.py 65687aa
> tools/marvin/marvin/marvinPlugin.py c817cd6
>
> Diff: https://reviews.apache.org/r/21070/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 21070: Fixed few exception cases.
Posted by sanjeev n <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21070/#review42157
-----------------------------------------------------------
Ship it!
Ship It!
- sanjeev n
On May 5, 2014, 6:49 a.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21070/
> -----------------------------------------------------------
>
> (Updated May 5, 2014, 6:49 a.m.)
>
>
> Review request for cloudstack and sanjeev n.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed few exception cases. When some request fails, it throws up the stack, but the message str..does not contains a given attribute is little misleading. So, raising exceptions for all cases.
> Added testsuite name for userprovided\default log folder path
> Fixed pep8 changes.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/cloudstackConnection.py caa8609
> tools/marvin/marvin/cloudstackTestClient.py d3a6b94
> tools/marvin/marvin/configGenerator.py 4f03fd0
> tools/marvin/marvin/marvinInit.py de580ce
> tools/marvin/marvin/marvinLog.py 65687aa
> tools/marvin/marvin/marvinPlugin.py c817cd6
>
> Diff: https://reviews.apache.org/r/21070/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
RE: Review Request 21070: Fixed few exception cases.
Posted by Santhosh Edukulla <sa...@citrix.com>.
Some places we are explicitly verifying for exception, EX: with assert for exception, with assert for not exception from CS, at some places we are verifying for return value say failed. If we return failed, then cases above are not clear. So, made it uniform for raising exception. Even earlier, the command raises the exception caught for command failure case.
I believe all the test cases will still work. Because of returning failed, base.py classes are directly calling on operation on the returned response, assuming response has received. So, the message dumped when failure happens is little misleading, it says string object has no attribute, but actual exception was some thing different.
During phase2 changes, we can make it more deterministic, by returning failure to test code, they can verify and assert, and we as well provide error information to them.
Santhosh
________________________________________
From: Gaurav Aradhye [gaurav.aradhye@clogeny.com]
Sent: Monday, May 05, 2014 3:04 AM
To: dev@cloudstack.apache.org; Santhosh Edukulla
Cc: Sanjeev Neelarapu
Subject: Re: Review Request 21070: Fixed few exception cases.
Hi Santhosh,
I would like to mention one concern here.
If we are raising exception through "marvinRequest" function in cloudstackConnection.py, instead of returning FAILED, then this will need many test cases to be changed too, where we are explicitly asserting the output of particular marvin command as FAILED or not as FAILED. Do you think this can be avoided?
Many test cases failed last time we changed raising exception to returning FAILED. And those fixes will stop working in this case.
May be we can solve the problem by extracting the exception message and raising it as a string e.g raise Exception(string), and this will be logged to the failure logs, and ultimately we return FAILED.
Regards,
Gaurav
On Mon, May 5, 2014 at 12:19 PM, Santhosh Edukulla <sa...@citrix.com>> wrote:
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21070/
-----------------------------------------------------------
Review request for cloudstack and sanjeev n.
Repository: cloudstack-git
Description
-------
Fixed few exception cases. When some request fails, it throws up the stack, but the message str..does not contains a given attribute is little misleading. So, raising exceptions for all cases.
Added testsuite name for userprovided\default log folder path
Fixed pep8 changes.
Diffs
-----
tools/marvin/marvin/cloudstackConnection.py caa8609
tools/marvin/marvin/cloudstackTestClient.py d3a6b94
tools/marvin/marvin/configGenerator.py 4f03fd0
tools/marvin/marvin/marvinInit.py de580ce
tools/marvin/marvin/marvinLog.py 65687aa
tools/marvin/marvin/marvinPlugin.py c817cd6
Diff: https://reviews.apache.org/r/21070/diff/
Testing
-------
Thanks,
Santhosh Edukulla
Re: Review Request 21070: Fixed few exception cases.
Posted by Gaurav Aradhye <ga...@clogeny.com>.
Hi Santhosh,
I would like to mention one concern here.
If we are raising exception through "marvinRequest" function in
cloudstackConnection.py, instead of returning FAILED, then this will need
many test cases to be changed too, where we are explicitly asserting the
output of particular marvin command as FAILED or not as FAILED. Do you
think this can be avoided?
Many test cases failed last time we changed raising exception to returning
FAILED. And those fixes will stop working in this case.
May be we can solve the problem by extracting the exception message and
raising it as a string e.g raise Exception(string), and this will be logged
to the failure logs, and ultimately we return FAILED.
Regards,
Gaurav
On Mon, May 5, 2014 at 12:19 PM, Santhosh Edukulla <
santhosh.edukulla@citrix.com> wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21070/
> -----------------------------------------------------------
>
> Review request for cloudstack and sanjeev n.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed few exception cases. When some request fails, it throws up the
> stack, but the message str..does not contains a given attribute is little
> misleading. So, raising exceptions for all cases.
> Added testsuite name for userprovided\default log folder path
> Fixed pep8 changes.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/cloudstackConnection.py caa8609
> tools/marvin/marvin/cloudstackTestClient.py d3a6b94
> tools/marvin/marvin/configGenerator.py 4f03fd0
> tools/marvin/marvin/marvinInit.py de580ce
> tools/marvin/marvin/marvinLog.py 65687aa
> tools/marvin/marvin/marvinPlugin.py c817cd6
>
> Diff: https://reviews.apache.org/r/21070/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 21070: Fixed few exception cases.
Posted by SrikanteswaraRao Talluri <sr...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21070/#review42955
-----------------------------------------------------------
pushed to master
24bf1c56dfe437766a716dc8ced6d892782bd6a1
- SrikanteswaraRao Talluri
On May 5, 2014, 6:49 a.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21070/
> -----------------------------------------------------------
>
> (Updated May 5, 2014, 6:49 a.m.)
>
>
> Review request for cloudstack and sanjeev n.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed few exception cases. When some request fails, it throws up the stack, but the message str..does not contains a given attribute is little misleading. So, raising exceptions for all cases.
> Added testsuite name for userprovided\default log folder path
> Fixed pep8 changes.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/cloudstackConnection.py caa8609
> tools/marvin/marvin/cloudstackTestClient.py d3a6b94
> tools/marvin/marvin/configGenerator.py 4f03fd0
> tools/marvin/marvin/marvinInit.py de580ce
> tools/marvin/marvin/marvinLog.py 65687aa
> tools/marvin/marvin/marvinPlugin.py c817cd6
>
> Diff: https://reviews.apache.org/r/21070/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Santhosh Edukulla
>
>