You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by John Dilley <jo...@citrix.com> on 2014/08/06 12:07:21 UTC

Review Request 24378: CLOUDSTACK-7268: Ignore "already exists" error in createEgressFirewallRule

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

Review request for cloudstack and Santhosh Edukulla.


Bugs: CLOUDSTACK-7268
    https://issues.apache.org/jira/browse/CLOUDSTACK-7268


Repository: cloudstack-git


Description
-------

Although we could search for an existing firewall rule, this fix is simpler, and also less prone to race conditions if multiple threads are running.


Diffs
-----

  tools/marvin/marvin/lib/base.py 3a1f7e6 

Diff: https://reviews.apache.org/r/24378/diff/


Testing
-------

Tested on KVM advanced zone


Thanks,

John Dilley


Re: Review Request 24378: CLOUDSTACK-7268: Ignore "already exists" error in createEgressFirewallRule

Posted by Santhosh Edukulla <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24378/#review49884
-----------------------------------------------------------

Ship it!


Ship It!

- Santhosh Edukulla


On Aug. 7, 2014, 7:41 a.m., John Dilley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24378/
> -----------------------------------------------------------
> 
> (Updated Aug. 7, 2014, 7:41 a.m.)
> 
> 
> Review request for cloudstack and Santhosh Edukulla.
> 
> 
> Bugs: CLOUDSTACK-7268
>     https://issues.apache.org/jira/browse/CLOUDSTACK-7268
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Although we could search for an existing firewall rule, this fix is simpler, and also less prone to race conditions if multiple threads are running.
> 
> 
> Diffs
> -----
> 
>   tools/marvin/marvin/lib/base.py 3a1f7e6 
> 
> Diff: https://reviews.apache.org/r/24378/diff/
> 
> 
> Testing
> -------
> 
> Tested on KVM advanced zone
> 
> 
> Thanks,
> 
> John Dilley
> 
>


Re: Review Request 24378: CLOUDSTACK-7268: Ignore "already exists" error in createEgressFirewallRule

Posted by ASF Subversion and Git Services <as...@urd.zones.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24378/#review49885
-----------------------------------------------------------


Commit 7ff7e9cf5ae4d5ababa0bf7e7ccb4a8cb1064045 in cloudstack's branch refs/heads/master from John Dilley
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=7ff7e9c ]

CLOUDSTACK-7268: Ignore "already exists" error in createEgressFirewallRule

Signed-off-by: Santhosh Edukulla <sa...@gmail.com>


- ASF Subversion and Git Services


On Aug. 7, 2014, 7:41 a.m., John Dilley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24378/
> -----------------------------------------------------------
> 
> (Updated Aug. 7, 2014, 7:41 a.m.)
> 
> 
> Review request for cloudstack and Santhosh Edukulla.
> 
> 
> Bugs: CLOUDSTACK-7268
>     https://issues.apache.org/jira/browse/CLOUDSTACK-7268
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Although we could search for an existing firewall rule, this fix is simpler, and also less prone to race conditions if multiple threads are running.
> 
> 
> Diffs
> -----
> 
>   tools/marvin/marvin/lib/base.py 3a1f7e6 
> 
> Diff: https://reviews.apache.org/r/24378/diff/
> 
> 
> Testing
> -------
> 
> Tested on KVM advanced zone
> 
> 
> Thanks,
> 
> John Dilley
> 
>


Re: Review Request 24378: CLOUDSTACK-7268: Ignore "already exists" error in createEgressFirewallRule

Posted by John Dilley <jo...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24378/
-----------------------------------------------------------

(Updated Aug. 7, 2014, 7:41 a.m.)


Review request for cloudstack and Santhosh Edukulla.


Bugs: CLOUDSTACK-7268
    https://issues.apache.org/jira/browse/CLOUDSTACK-7268


Repository: cloudstack-git


Description
-------

Although we could search for an existing firewall rule, this fix is simpler, and also less prone to race conditions if multiple threads are running.


Diffs (updated)
-----

  tools/marvin/marvin/lib/base.py 3a1f7e6 

Diff: https://reviews.apache.org/r/24378/diff/


Testing
-------

Tested on KVM advanced zone


Thanks,

John Dilley


Re: Review Request 24378: CLOUDSTACK-7268: Ignore "already exists" error in createEgressFirewallRule

Posted by John Dilley <jo...@citrix.com>.

> On Aug. 7, 2014, 7:21 a.m., Santhosh Edukulla wrote:
> > tools/marvin/marvin/lib/base.py, line 350
> > <https://reviews.apache.org/r/24378/diff/1/?file=653687#file653687line350>
> >
> >     may be to keep simple check if not string in e.. and raise, pass can be removed. As well, is the exception thrown is only cloudstackAPIException or some other type? add a default handler if possible and fail in case so?
> >     
> >     Do a lower comparison of message again for both left and right hand vlaue.

The failure we want to ignore will always be cloudstackAPI exception. Any other type of exception will not match an except block, so will be raised rather than handled.


- John


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


On Aug. 7, 2014, 7:41 a.m., John Dilley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24378/
> -----------------------------------------------------------
> 
> (Updated Aug. 7, 2014, 7:41 a.m.)
> 
> 
> Review request for cloudstack and Santhosh Edukulla.
> 
> 
> Bugs: CLOUDSTACK-7268
>     https://issues.apache.org/jira/browse/CLOUDSTACK-7268
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Although we could search for an existing firewall rule, this fix is simpler, and also less prone to race conditions if multiple threads are running.
> 
> 
> Diffs
> -----
> 
>   tools/marvin/marvin/lib/base.py 3a1f7e6 
> 
> Diff: https://reviews.apache.org/r/24378/diff/
> 
> 
> Testing
> -------
> 
> Tested on KVM advanced zone
> 
> 
> Thanks,
> 
> John Dilley
> 
>


Re: Review Request 24378: CLOUDSTACK-7268: Ignore "already exists" error in createEgressFirewallRule

Posted by Santhosh Edukulla <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24378/#review49877
-----------------------------------------------------------



tools/marvin/marvin/lib/base.py
<https://reviews.apache.org/r/24378/#comment87278>

    may be to keep simple check if not string in e.. and raise, pass can be removed. As well, is the exception thrown is only cloudstackAPIException or some other type? add a default handler if possible and fail in case so?
    
    Do a lower comparison of message again for both left and right hand vlaue.


- Santhosh Edukulla


On Aug. 6, 2014, 2:58 p.m., John Dilley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24378/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2014, 2:58 p.m.)
> 
> 
> Review request for cloudstack and Santhosh Edukulla.
> 
> 
> Bugs: CLOUDSTACK-7268
>     https://issues.apache.org/jira/browse/CLOUDSTACK-7268
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Although we could search for an existing firewall rule, this fix is simpler, and also less prone to race conditions if multiple threads are running.
> 
> 
> Diffs
> -----
> 
>   tools/marvin/marvin/lib/base.py 3a1f7e6 
> 
> Diff: https://reviews.apache.org/r/24378/diff/
> 
> 
> Testing
> -------
> 
> Tested on KVM advanced zone
> 
> 
> Thanks,
> 
> John Dilley
> 
>


Re: Review Request 24378: CLOUDSTACK-7268: Ignore "already exists" error in createEgressFirewallRule

Posted by John Dilley <jo...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24378/
-----------------------------------------------------------

(Updated Aug. 6, 2014, 2:58 p.m.)


Review request for cloudstack and Santhosh Edukulla.


Bugs: CLOUDSTACK-7268
    https://issues.apache.org/jira/browse/CLOUDSTACK-7268


Repository: cloudstack-git


Description
-------

Although we could search for an existing firewall rule, this fix is simpler, and also less prone to race conditions if multiple threads are running.


Diffs
-----

  tools/marvin/marvin/lib/base.py 3a1f7e6 

Diff: https://reviews.apache.org/r/24378/diff/


Testing
-------

Tested on KVM advanced zone


Thanks,

John Dilley