You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Miguel Ferreira <mi...@me.com> on 2014/07/23 13:56:11 UTC
Review Request 23847: Check if config specifies zones before iterating
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23847/
-----------------------------------------------------------
Review request for cloudstack, daan Hoogland, Santhosh Edukulla, and Hugo Trippaers.
Repository: cloudstack-git
Description
-------
Running a marvin test that does not specify a zone in the config file will produce an exception while parsing the config:
Exception Occurred Under init ['Traceback (most recent call last):\n', ' File "/usr/local/lib/python2.7/site-packages/marvin/marvinInit.py", line 118, in __setHypervisorAndZoneInfo\n for zone in self.__parsedConfig.zones:\n', "TypeError: 'NoneType' object is not iterable\n"]
This happens when the code starts iterating over a list of zones that does not exist.
I've added a check for the existence of the list of zones before iterating.
Diffs
-----
tools/marvin/marvin/marvinInit.py ce9b43f
Diff: https://reviews.apache.org/r/23847/diff/
Testing
-------
Tests now run without the mentioned exception
Thanks,
Miguel Ferreira
Re: Review Request 23847: Check if config specifies zones before iterating
Posted by Santhosh Edukulla <sa...@citrix.com>.
> On July 23, 2014, 12:05 p.m., Santhosh Edukulla wrote:
> > tools/marvin/marvin/marvinInit.py, line 114
> > <https://reviews.apache.org/r/23847/diff/1/?file=640532#file640532line114>
> >
> > please use is not None, rather !=
>
> Miguel Ferreira wrote:
> I will do this this time, but if something is broken, I fix it and you see better ways of fixing it, please be my guest and do it yourself. after all that is the spirit of open-source, right?
sure, but lets make the submitter realize some better ways if there are, after all it helps the review submission and cleaner approach Agree? The review iam doing signifies its open source for your submission. Functionally, modifying your review submission is like altering the behavior and may add bugs, also resubmitting my review again.
- Santhosh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23847/#review48496
-----------------------------------------------------------
On July 23, 2014, 12:10 p.m., Miguel Ferreira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23847/
> -----------------------------------------------------------
>
> (Updated July 23, 2014, 12:10 p.m.)
>
>
> Review request for cloudstack, daan Hoogland, Santhosh Edukulla, and Hugo Trippaers.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Running a marvin test that does not specify a zone in the config file will produce an exception while parsing the config:
>
> Exception Occurred Under init ['Traceback (most recent call last):\n', ' File "/usr/local/lib/python2.7/site-packages/marvin/marvinInit.py", line 118, in __setHypervisorAndZoneInfo\n for zone in self.__parsedConfig.zones:\n', "TypeError: 'NoneType' object is not iterable\n"]
>
> This happens when the code starts iterating over a list of zones that does not exist.
> I've added a check for the existence of the list of zones before iterating.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/marvinInit.py ce9b43f
>
> Diff: https://reviews.apache.org/r/23847/diff/
>
>
> Testing
> -------
>
> Tests now run without the mentioned exception
>
>
> Thanks,
>
> Miguel Ferreira
>
>
Re: Review Request 23847: Check if config specifies zones before iterating
Posted by Miguel Ferreira <mi...@me.com>.
> On July 23, 2014, 12:05 p.m., Santhosh Edukulla wrote:
> > tools/marvin/marvin/marvinInit.py, line 114
> > <https://reviews.apache.org/r/23847/diff/1/?file=640532#file640532line114>
> >
> > please use is not None, rather !=
>
> Miguel Ferreira wrote:
> I will do this this time, but if something is broken, I fix it and you see better ways of fixing it, please be my guest and do it yourself. after all that is the spirit of open-source, right?
>
> Santhosh Edukulla wrote:
> sure, but lets make the submitter realize some better ways if there are, after all it helps the review submission and cleaner approach Agree? The review iam doing signifies its open source for your submission. Functionally, modifying your review submission is like altering the behavior and may add bugs, also resubmitting my review again.
>
> Miguel Ferreira wrote:
> Patch was not in. Will submit it again.
Of course that the reviewer has the responsibility to make sure that the submitter adheres to the guidelines of a project. But the reviewer is not the manager of the submitter. I could even understand if this patch (and others I've submitted) where about new features. But they are not. They are fixing things that are broken.
If I decide to take the time to contribute back what I do to fix the code in the project, and address bugs in the system, I do not expect to get push back based on "there are better ways to do it". Because it was broken to begin with, so my patch is in fact a better way to do it. I'll be very happy if you say, sorry but I don't accept your patch because I know better ways to fix the code and I will do it myself.
However, what you are doing is leveraging my time to get things done the way you see best, and again, I'm not here to be managed.
- Miguel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23847/#review48496
-----------------------------------------------------------
On July 23, 2014, 12:17 p.m., Miguel Ferreira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23847/
> -----------------------------------------------------------
>
> (Updated July 23, 2014, 12:17 p.m.)
>
>
> Review request for cloudstack, daan Hoogland, Santhosh Edukulla, and Hugo Trippaers.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Running a marvin test that does not specify a zone in the config file will produce an exception while parsing the config:
>
> Exception Occurred Under init ['Traceback (most recent call last):\n', ' File "/usr/local/lib/python2.7/site-packages/marvin/marvinInit.py", line 118, in __setHypervisorAndZoneInfo\n for zone in self.__parsedConfig.zones:\n', "TypeError: 'NoneType' object is not iterable\n"]
>
> This happens when the code starts iterating over a list of zones that does not exist.
> I've added a check for the existence of the list of zones before iterating.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/marvinInit.py ce9b43f
>
> Diff: https://reviews.apache.org/r/23847/diff/
>
>
> Testing
> -------
>
> Tests now run without the mentioned exception
>
>
> Thanks,
>
> Miguel Ferreira
>
>
Re: Review Request 23847: Check if config specifies zones before iterating
Posted by Miguel Ferreira <mi...@me.com>.
> On July 23, 2014, 12:05 p.m., Santhosh Edukulla wrote:
> > tools/marvin/marvin/marvinInit.py, line 114
> > <https://reviews.apache.org/r/23847/diff/1/?file=640532#file640532line114>
> >
> > please use is not None, rather !=
I will do this this time, but if something is broken, I fix it and you see better ways of fixing it, please be my guest and do it yourself. after all that is the spirit of open-source, right?
- Miguel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23847/#review48496
-----------------------------------------------------------
On July 23, 2014, 11:56 a.m., Miguel Ferreira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23847/
> -----------------------------------------------------------
>
> (Updated July 23, 2014, 11:56 a.m.)
>
>
> Review request for cloudstack, daan Hoogland, Santhosh Edukulla, and Hugo Trippaers.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Running a marvin test that does not specify a zone in the config file will produce an exception while parsing the config:
>
> Exception Occurred Under init ['Traceback (most recent call last):\n', ' File "/usr/local/lib/python2.7/site-packages/marvin/marvinInit.py", line 118, in __setHypervisorAndZoneInfo\n for zone in self.__parsedConfig.zones:\n', "TypeError: 'NoneType' object is not iterable\n"]
>
> This happens when the code starts iterating over a list of zones that does not exist.
> I've added a check for the existence of the list of zones before iterating.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/marvinInit.py ce9b43f
>
> Diff: https://reviews.apache.org/r/23847/diff/
>
>
> Testing
> -------
>
> Tests now run without the mentioned exception
>
>
> Thanks,
>
> Miguel Ferreira
>
>
Re: Review Request 23847: Check if config specifies zones before iterating
Posted by Miguel Ferreira <mi...@me.com>.
> On July 23, 2014, 12:05 p.m., Santhosh Edukulla wrote:
> > tools/marvin/marvin/marvinInit.py, line 114
> > <https://reviews.apache.org/r/23847/diff/1/?file=640532#file640532line114>
> >
> > please use is not None, rather !=
>
> Miguel Ferreira wrote:
> I will do this this time, but if something is broken, I fix it and you see better ways of fixing it, please be my guest and do it yourself. after all that is the spirit of open-source, right?
>
> Santhosh Edukulla wrote:
> sure, but lets make the submitter realize some better ways if there are, after all it helps the review submission and cleaner approach Agree? The review iam doing signifies its open source for your submission. Functionally, modifying your review submission is like altering the behavior and may add bugs, also resubmitting my review again.
Patch was not in. Will submit it again.
- Miguel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23847/#review48496
-----------------------------------------------------------
On July 23, 2014, 12:17 p.m., Miguel Ferreira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23847/
> -----------------------------------------------------------
>
> (Updated July 23, 2014, 12:17 p.m.)
>
>
> Review request for cloudstack, daan Hoogland, Santhosh Edukulla, and Hugo Trippaers.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Running a marvin test that does not specify a zone in the config file will produce an exception while parsing the config:
>
> Exception Occurred Under init ['Traceback (most recent call last):\n', ' File "/usr/local/lib/python2.7/site-packages/marvin/marvinInit.py", line 118, in __setHypervisorAndZoneInfo\n for zone in self.__parsedConfig.zones:\n', "TypeError: 'NoneType' object is not iterable\n"]
>
> This happens when the code starts iterating over a list of zones that does not exist.
> I've added a check for the existence of the list of zones before iterating.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/marvinInit.py ce9b43f
>
> Diff: https://reviews.apache.org/r/23847/diff/
>
>
> Testing
> -------
>
> Tests now run without the mentioned exception
>
>
> Thanks,
>
> Miguel Ferreira
>
>
Re: Review Request 23847: Check if config specifies zones before iterating
Posted by Santhosh Edukulla <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23847/#review48496
-----------------------------------------------------------
tools/marvin/marvin/marvinInit.py
<https://reviews.apache.org/r/23847/#comment85134>
please use is not None, rather !=
- Santhosh Edukulla
On July 23, 2014, 11:56 a.m., Miguel Ferreira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23847/
> -----------------------------------------------------------
>
> (Updated July 23, 2014, 11:56 a.m.)
>
>
> Review request for cloudstack, daan Hoogland, Santhosh Edukulla, and Hugo Trippaers.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Running a marvin test that does not specify a zone in the config file will produce an exception while parsing the config:
>
> Exception Occurred Under init ['Traceback (most recent call last):\n', ' File "/usr/local/lib/python2.7/site-packages/marvin/marvinInit.py", line 118, in __setHypervisorAndZoneInfo\n for zone in self.__parsedConfig.zones:\n', "TypeError: 'NoneType' object is not iterable\n"]
>
> This happens when the code starts iterating over a list of zones that does not exist.
> I've added a check for the existence of the list of zones before iterating.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/marvinInit.py ce9b43f
>
> Diff: https://reviews.apache.org/r/23847/diff/
>
>
> Testing
> -------
>
> Tests now run without the mentioned exception
>
>
> Thanks,
>
> Miguel Ferreira
>
>
Re: Review Request 23847: Check if config specifies zones before iterating
Posted by Miguel Ferreira <mi...@me.com>.
> On July 23, 2014, 2:58 p.m., Santhosh Edukulla wrote:
> > Ship It!
>
> Santhosh Edukulla wrote:
> Pushed the patch to master: 9f4f921..bb1c70b master -> master
>
> One small note though. Please add branch and bug information to review requests.
>
Sure. Thanks
- Miguel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23847/#review48509
-----------------------------------------------------------
On July 23, 2014, 12:17 p.m., Miguel Ferreira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23847/
> -----------------------------------------------------------
>
> (Updated July 23, 2014, 12:17 p.m.)
>
>
> Review request for cloudstack, daan Hoogland, Santhosh Edukulla, and Hugo Trippaers.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Running a marvin test that does not specify a zone in the config file will produce an exception while parsing the config:
>
> Exception Occurred Under init ['Traceback (most recent call last):\n', ' File "/usr/local/lib/python2.7/site-packages/marvin/marvinInit.py", line 118, in __setHypervisorAndZoneInfo\n for zone in self.__parsedConfig.zones:\n', "TypeError: 'NoneType' object is not iterable\n"]
>
> This happens when the code starts iterating over a list of zones that does not exist.
> I've added a check for the existence of the list of zones before iterating.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/marvinInit.py ce9b43f
>
> Diff: https://reviews.apache.org/r/23847/diff/
>
>
> Testing
> -------
>
> Tests now run without the mentioned exception
>
>
> Thanks,
>
> Miguel Ferreira
>
>
Re: Review Request 23847: Check if config specifies zones before iterating
Posted by Santhosh Edukulla <sa...@citrix.com>.
> On July 23, 2014, 2:58 p.m., Santhosh Edukulla wrote:
> > Ship It!
Pushed the patch to master: 9f4f921..bb1c70b master -> master
One small note though. Please add branch and bug information to review requests.
- Santhosh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23847/#review48509
-----------------------------------------------------------
On July 23, 2014, 12:17 p.m., Miguel Ferreira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23847/
> -----------------------------------------------------------
>
> (Updated July 23, 2014, 12:17 p.m.)
>
>
> Review request for cloudstack, daan Hoogland, Santhosh Edukulla, and Hugo Trippaers.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Running a marvin test that does not specify a zone in the config file will produce an exception while parsing the config:
>
> Exception Occurred Under init ['Traceback (most recent call last):\n', ' File "/usr/local/lib/python2.7/site-packages/marvin/marvinInit.py", line 118, in __setHypervisorAndZoneInfo\n for zone in self.__parsedConfig.zones:\n', "TypeError: 'NoneType' object is not iterable\n"]
>
> This happens when the code starts iterating over a list of zones that does not exist.
> I've added a check for the existence of the list of zones before iterating.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/marvinInit.py ce9b43f
>
> Diff: https://reviews.apache.org/r/23847/diff/
>
>
> Testing
> -------
>
> Tests now run without the mentioned exception
>
>
> Thanks,
>
> Miguel Ferreira
>
>
Re: Review Request 23847: Check if config specifies zones before iterating
Posted by Santhosh Edukulla <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23847/#review48509
-----------------------------------------------------------
Ship it!
Ship It!
- Santhosh Edukulla
On July 23, 2014, 12:17 p.m., Miguel Ferreira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23847/
> -----------------------------------------------------------
>
> (Updated July 23, 2014, 12:17 p.m.)
>
>
> Review request for cloudstack, daan Hoogland, Santhosh Edukulla, and Hugo Trippaers.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Running a marvin test that does not specify a zone in the config file will produce an exception while parsing the config:
>
> Exception Occurred Under init ['Traceback (most recent call last):\n', ' File "/usr/local/lib/python2.7/site-packages/marvin/marvinInit.py", line 118, in __setHypervisorAndZoneInfo\n for zone in self.__parsedConfig.zones:\n', "TypeError: 'NoneType' object is not iterable\n"]
>
> This happens when the code starts iterating over a list of zones that does not exist.
> I've added a check for the existence of the list of zones before iterating.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/marvinInit.py ce9b43f
>
> Diff: https://reviews.apache.org/r/23847/diff/
>
>
> Testing
> -------
>
> Tests now run without the mentioned exception
>
>
> Thanks,
>
> Miguel Ferreira
>
>
Re: Review Request 23847: Check if config specifies zones before iterating
Posted by Miguel Ferreira <mi...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23847/
-----------------------------------------------------------
(Updated July 23, 2014, 12:17 p.m.)
Review request for cloudstack, daan Hoogland, Santhosh Edukulla, and Hugo Trippaers.
Changes
-------
Update diff to address remark (previous update sent the initial patch)
Repository: cloudstack-git
Description
-------
Running a marvin test that does not specify a zone in the config file will produce an exception while parsing the config:
Exception Occurred Under init ['Traceback (most recent call last):\n', ' File "/usr/local/lib/python2.7/site-packages/marvin/marvinInit.py", line 118, in __setHypervisorAndZoneInfo\n for zone in self.__parsedConfig.zones:\n', "TypeError: 'NoneType' object is not iterable\n"]
This happens when the code starts iterating over a list of zones that does not exist.
I've added a check for the existence of the list of zones before iterating.
Diffs (updated)
-----
tools/marvin/marvin/marvinInit.py ce9b43f
Diff: https://reviews.apache.org/r/23847/diff/
Testing
-------
Tests now run without the mentioned exception
Thanks,
Miguel Ferreira
Re: Review Request 23847: Check if config specifies zones before iterating
Posted by Miguel Ferreira <mi...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23847/
-----------------------------------------------------------
(Updated July 23, 2014, 12:10 p.m.)
Review request for cloudstack, daan Hoogland, Santhosh Edukulla, and Hugo Trippaers.
Changes
-------
Address remark
Repository: cloudstack-git
Description
-------
Running a marvin test that does not specify a zone in the config file will produce an exception while parsing the config:
Exception Occurred Under init ['Traceback (most recent call last):\n', ' File "/usr/local/lib/python2.7/site-packages/marvin/marvinInit.py", line 118, in __setHypervisorAndZoneInfo\n for zone in self.__parsedConfig.zones:\n', "TypeError: 'NoneType' object is not iterable\n"]
This happens when the code starts iterating over a list of zones that does not exist.
I've added a check for the existence of the list of zones before iterating.
Diffs (updated)
-----
tools/marvin/marvin/marvinInit.py ce9b43f
Diff: https://reviews.apache.org/r/23847/diff/
Testing
-------
Tests now run without the mentioned exception
Thanks,
Miguel Ferreira