Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs
Hi Daan / Alex, Sorry I missed this change at the time, but it looks problematic to me. The code is trying to check for a scheme part in the incoming value, and doesn't add a scheme if value already has one. Therefore, if the scheme check has a false positive (it thinks the value has a scheme, but it doesn't), we are guaranteed to produce an invalid URI (no scheme part) and an exception. I'm pretty sure the current scheme check is invalid given the URI spec, but definitely shout if I'm off base. Here's the check: if (value.toString().contains(:)) return new URI(value.toString()); else return new URI(scheme, value.toString(), null); value in this case is ssp, the Scheme Specific Part (see signature of the URI constructor we're using [1]). There are basically no restrictions on the scheme specific part of a URI, and certainly no restriction on the presence of colons. In fact, many URIs have an authority part in the SSP (e.g. user:password) which *requires* a colon [2]. http is another example, where a colon is used within the SSP to specify the port. In summary, it's perfectly valid to have a colon in the scheme specific part of a URI, so we shouldn't be creating invalid URIs in that case. What are we trying to protect against in the first place with the check? Should we just remove the check? Thanks, Dave. [1] http://docs.oracle.com/javase/6/docs/api/java/net/URI.html#URI(java.lang.String, java.lang.String, java.lang.String) [2] http://www.ietf.org/rfc/rfc2396.txt
Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs
Hi, Adding Daan's reply - I sent to the wrong dev@ first time around, so the reply went astray: On Tue, Aug 6, 2013 at 10:30 AM, Dave Cahill dcah...@midokura.com wrote: I'm pretty sure the current scheme check is invalid given the URI spec, but definitely shout if I'm off base. You are not 'way' of but please review https://reviews.apache.org/r/12849/ If the caller needs BroadcastDomainType the reason why should be known and the value-member can be called taking your foreseen problem into account. The code you are quoting would 'just' be a default behavior. regards, Daan If you're around, maybe discussing on IRC would be faster? I took a read of the code you posted, and the default behavior still causes an exception if colons are present in the value passed to toUri. I might be interpreting wrong, but it sounds like you're saying anyone using BroadcastDomainType should actively work around the new (incorrect) default colon handling by changing what value they're passing. This seems like a strange choice, since the value they're passing may be perfectly correct and other parts of the system may depend on it. Why would we intentionally change the existing behavior to start handling colons incorrectly, and put the burden on the caller? The rest of the code looks fine to me, but the colon part breaks existing callers (the MidoNet plugin for one), is confusing for anyone using BroadcastDomainType in future, and I don't yet see what it gains us. I'll add comments on the review. Thanks for the quick reply; feel free to grab me on IRC, we'll probably figure this out faster than on a mail thread. :) Thanks, Dave. On Tue, Aug 6, 2013 at 5:38 PM, Dave Cahill dcah...@midokura.com wrote: Hi Daan / Alex, Sorry I missed this change at the time, but it looks problematic to me. The code is trying to check for a scheme part in the incoming value, and doesn't add a scheme if value already has one. Therefore, if the scheme check has a false positive (it thinks the value has a scheme, but it doesn't), we are guaranteed to produce an invalid URI (no scheme part) and an exception. I'm pretty sure the current scheme check is invalid given the URI spec, but definitely shout if I'm off base. Here's the check: if (value.toString().contains(:)) return new URI(value.toString()); else return new URI(scheme, value.toString(), null); value in this case is ssp, the Scheme Specific Part (see signature of the URI constructor we're using [1]). There are basically no restrictions on the scheme specific part of a URI, and certainly no restriction on the presence of colons. In fact, many URIs have an authority part in the SSP (e.g. user:password) which *requires* a colon [2]. http is another example, where a colon is used within the SSP to specify the port. In summary, it's perfectly valid to have a colon in the scheme specific part of a URI, so we shouldn't be creating invalid URIs in that case. What are we trying to protect against in the first place with the check? Should we just remove the check? Thanks, Dave. [1] http://docs.oracle.com/javase/6/docs/api/java/net/URI.html#URI(java.lang.String, java.lang.String, java.lang.String) [2] http://www.ietf.org/rfc/rfc2396.txt
RE: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs
Ok, I don't like changing this enum. I'd rather throw it out and start over but you are answering the question by sharing your views on school of programming, i think. BroadcastDomainType by its name implies it could be unknown but never undecided. Do you agree? I can comply with any school. Op 24 jul. 2013 19:19 schreef Alex Huang alex.hu...@citrix.com het volgende: Daan, ** ** Sorry for the late reply. Now, it kinda goes into code/design philosophy. I’ll tell you what mine is. ** ** There’s one school of thought that code should cover all bases. To me having undecided is in that school because it asks “could it be undecided” and the answer is of course yes because in cs everything is possible. ** ** The other is to say Undecided is never accepted because the code can never do anything with that. Having assert to tell developers that this is just not used in this manner and speaks louder and more accurate than an Undecided. I’m more in this school. I think it should ask the question “should it be undecided”. ** ** --Alex ** ** *From:* Daan Hoogland [mailto:daan.hoogl...@gmail.com] *Sent:* Sunday, July 21, 2013 8:37 AM *To:* Alex Huang *Cc:* Hugo Trippaers; cloudstack *Subject:* Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs ** ** Your remarks on the assert resembles a comment in the code that I didn't write ;) But seriously; If the string contains a scheme that is not in the enum, you want a exception to be thrown instead of UnDecided or UnDefined? regards, ** ** On Sun, Jul 21, 2013 at 5:32 PM, Alex Huang alex.hu...@citrix.com wrote: Daan, I’m not sure I understand what you’re saying here. --Alex *From:* Daan Hoogland [mailto:daan.hoogl...@gmail.com] *Sent:* Sunday, July 21, 2013 4:23 AM *To:* Alex Huang *Cc:* Hugo Trippaers; cloudstack *Subject:* Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs Alex, I saw you put you comments in the commit as comment...??? why not as code then? On Sat, Jul 20, 2013 at 3:42 PM, Alex Huang alex.hu...@citrix.com wrote: Daan, It should assert and throw a CloudRuntimeException. https://cwiki.apache.org/confluence/display/CLOUDSTACK/Exceptions+and+logging --Alex *From:* Daan Hoogland [mailto:daan.hoogl...@gmail.com] *Sent:* Saturday, July 20, 2013 3:25 AM *To:* Alex Huang *Cc:* Hugo Trippaers; cloudstack *Subject:* Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs Alex, it falls through to 'UnDecided' if not known. Do you mean it should throw something? or maybe add an extra 'UnDefined'? On Fri, Jul 19, 2013 at 5:31 PM, Alex Huang alex.hu...@citrix.com wrote: This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12685/ Ship it! master: 2d4464d One review comment is if you are checking for the colon to see if the value has the schema already, then shouldn't it also check if the schema matches what's declared? But that shouldn't block this review. The code itself is technically sound. - Alex Huang On July 17th, 2013, 3:46 p.m. UTC, daan Hoogland wrote: Review request for cloudstack and Hugo Trippaers. By daan Hoogland. *Updated July 17, 2013, 3:46 p.m.* *Bugs: *CLOUDSTACK-1532 *Repository: *cloudstack-git Description the review for the complete patch was open to long and no longer applicable, so I am starting with smaller patches as this is really needed to implement CLOUDSTACK-1532 Testing unit testing Diffs - api/src/com/cloud/network/Networks.java (5aede05) - api/test/com/cloud/network/NetworksTest.java (PRE-CREATION) View Diff https://reviews.apache.org/r/12685/diff/ ** **
RE: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs
Daan, Agreed. If it's unknown, the variable of that type can just be null which is an accepted practice for saying the business logic hasn't determined the value yet. --Alex From: Daan Hoogland [mailto:daan.hoogl...@gmail.com] Sent: Wednesday, July 24, 2013 12:41 PM To: Alex Huang Cc: cloudstack Subject: RE: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs Ok, I don't like changing this enum. I'd rather throw it out and start over but you are answering the question by sharing your views on school of programming, i think. BroadcastDomainType by its name implies it could be unknown but never undecided. Do you agree? I can comply with any school. Op 24 jul. 2013 19:19 schreef Alex Huang alex.hu...@citrix.commailto:alex.hu...@citrix.com het volgende: Daan, Sorry for the late reply. Now, it kinda goes into code/design philosophy. I'll tell you what mine is. There's one school of thought that code should cover all bases. To me having undecided is in that school because it asks could it be undecided and the answer is of course yes because in cs everything is possible. The other is to say Undecided is never accepted because the code can never do anything with that. Having assert to tell developers that this is just not used in this manner and speaks louder and more accurate than an Undecided. I'm more in this school. I think it should ask the question should it be undecided. --Alex From: Daan Hoogland [mailto:daan.hoogl...@gmail.commailto:daan.hoogl...@gmail.com] Sent: Sunday, July 21, 2013 8:37 AM To: Alex Huang Cc: Hugo Trippaers; cloudstack Subject: Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs Your remarks on the assert resembles a comment in the code that I didn't write ;) But seriously; If the string contains a scheme that is not in the enum, you want a exception to be thrown instead of UnDecided or UnDefined? regards, On Sun, Jul 21, 2013 at 5:32 PM, Alex Huang alex.hu...@citrix.commailto:alex.hu...@citrix.com wrote: Daan, I'm not sure I understand what you're saying here. --Alex From: Daan Hoogland [mailto:daan.hoogl...@gmail.commailto:daan.hoogl...@gmail.com] Sent: Sunday, July 21, 2013 4:23 AM To: Alex Huang Cc: Hugo Trippaers; cloudstack Subject: Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs Alex, I saw you put you comments in the commit as comment...??? why not as code then? On Sat, Jul 20, 2013 at 3:42 PM, Alex Huang alex.hu...@citrix.commailto:alex.hu...@citrix.com wrote: Daan, It should assert and throw a CloudRuntimeException. https://cwiki.apache.org/confluence/display/CLOUDSTACK/Exceptions+and+logging --Alex From: Daan Hoogland [mailto:daan.hoogl...@gmail.commailto:daan.hoogl...@gmail.com] Sent: Saturday, July 20, 2013 3:25 AM To: Alex Huang Cc: Hugo Trippaers; cloudstack Subject: Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs Alex, it falls through to 'UnDecided' if not known. Do you mean it should throw something? or maybe add an extra 'UnDefined'? On Fri, Jul 19, 2013 at 5:31 PM, Alex Huang alex.hu...@citrix.commailto:alex.hu...@citrix.com wrote: This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12685/ Ship it! master: 2d4464d One review comment is if you are checking for the colon to see if the value has the schema already, then shouldn't it also check if the schema matches what's declared? But that shouldn't block this review. The code itself is technically sound. - Alex Huang On July 17th, 2013, 3:46 p.m. UTC, daan Hoogland wrote: Review request for cloudstack and Hugo Trippaers. By daan Hoogland. Updated July 17, 2013, 3:46 p.m. Bugs: CLOUDSTACK-1532 Repository: cloudstack-git Description the review for the complete patch was open to long and no longer applicable, so I am starting with smaller patches as this is really needed to implement CLOUDSTACK-1532 Testing unit testing Diffs * api/src/com/cloud/network/Networks.java (5aede05) * api/test/com/cloud/network/NetworksTest.java (PRE-CREATION) View Diffhttps://reviews.apache.org/r/12685/diff/
Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs
Alex, I saw you put you comments in the commit as comment...??? why not as code then? On Sat, Jul 20, 2013 at 3:42 PM, Alex Huang alex.hu...@citrix.com wrote: Daan, ** ** It should assert and throw a CloudRuntimeException. ** ** https://cwiki.apache.org/confluence/display/CLOUDSTACK/Exceptions+and+logging ** ** --Alex ** ** *From:* Daan Hoogland [mailto:daan.hoogl...@gmail.com] *Sent:* Saturday, July 20, 2013 3:25 AM *To:* Alex Huang *Cc:* Hugo Trippaers; cloudstack *Subject:* Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs ** ** Alex, it falls through to 'UnDecided' if not known. Do you mean it should throw something? or maybe add an extra 'UnDefined'? ** ** On Fri, Jul 19, 2013 at 5:31 PM, Alex Huang alex.hu...@citrix.com wrote: This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12685/ ** ** Ship it! master: 2d4464d ** ** One review comment is if you are checking for the colon to see if the value has the schema already, then shouldn't it also check if the schema matches what's declared? But that shouldn't block this review. The code itself is technically sound. ** ** - Alex Huang ** ** On July 17th, 2013, 3:46 p.m. UTC, daan Hoogland wrote: Review request for cloudstack and Hugo Trippaers. By daan Hoogland. *Updated July 17, 2013, 3:46 p.m.* *Bugs: *CLOUDSTACK-1532 *Repository: *cloudstack-git Description the review for the complete patch was open to long and no longer applicable, so I am starting with smaller patches as this is really needed to implement CLOUDSTACK-1532 Testing unit testing Diffs ** ** - api/src/com/cloud/network/Networks.java (5aede05) - api/test/com/cloud/network/NetworksTest.java (PRE-CREATION) View Diff https://reviews.apache.org/r/12685/diff/ ** **
RE: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs
Daan, I'm not sure I understand what you're saying here. --Alex From: Daan Hoogland [mailto:daan.hoogl...@gmail.com] Sent: Sunday, July 21, 2013 4:23 AM To: Alex Huang Cc: Hugo Trippaers; cloudstack Subject: Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs Alex, I saw you put you comments in the commit as comment...??? why not as code then? On Sat, Jul 20, 2013 at 3:42 PM, Alex Huang alex.hu...@citrix.commailto:alex.hu...@citrix.com wrote: Daan, It should assert and throw a CloudRuntimeException. https://cwiki.apache.org/confluence/display/CLOUDSTACK/Exceptions+and+logging --Alex From: Daan Hoogland [mailto:daan.hoogl...@gmail.commailto:daan.hoogl...@gmail.com] Sent: Saturday, July 20, 2013 3:25 AM To: Alex Huang Cc: Hugo Trippaers; cloudstack Subject: Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs Alex, it falls through to 'UnDecided' if not known. Do you mean it should throw something? or maybe add an extra 'UnDefined'? On Fri, Jul 19, 2013 at 5:31 PM, Alex Huang alex.hu...@citrix.commailto:alex.hu...@citrix.com wrote: This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12685/ Ship it! master: 2d4464d One review comment is if you are checking for the colon to see if the value has the schema already, then shouldn't it also check if the schema matches what's declared? But that shouldn't block this review. The code itself is technically sound. - Alex Huang On July 17th, 2013, 3:46 p.m. UTC, daan Hoogland wrote: Review request for cloudstack and Hugo Trippaers. By daan Hoogland. Updated July 17, 2013, 3:46 p.m. Bugs: CLOUDSTACK-1532 Repository: cloudstack-git Description the review for the complete patch was open to long and no longer applicable, so I am starting with smaller patches as this is really needed to implement CLOUDSTACK-1532 Testing unit testing Diffs * api/src/com/cloud/network/Networks.java (5aede05) * api/test/com/cloud/network/NetworksTest.java (PRE-CREATION) View Diffhttps://reviews.apache.org/r/12685/diff/
Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs
Your remarks on the assert resembles a comment in the code that I didn't write ;) But seriously; If the string contains a scheme that is not in the enum, you want a exception to be thrown instead of UnDecided or UnDefined? regards, On Sun, Jul 21, 2013 at 5:32 PM, Alex Huang alex.hu...@citrix.com wrote: Daan, ** ** I’m not sure I understand what you’re saying here. ** ** --Alex ** ** *From:* Daan Hoogland [mailto:daan.hoogl...@gmail.com] *Sent:* Sunday, July 21, 2013 4:23 AM *To:* Alex Huang *Cc:* Hugo Trippaers; cloudstack *Subject:* Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs ** ** Alex, I saw you put you comments in the commit as comment...??? why not as code then? ** ** On Sat, Jul 20, 2013 at 3:42 PM, Alex Huang alex.hu...@citrix.com wrote: Daan, It should assert and throw a CloudRuntimeException. https://cwiki.apache.org/confluence/display/CLOUDSTACK/Exceptions+and+logging --Alex *From:* Daan Hoogland [mailto:daan.hoogl...@gmail.com] *Sent:* Saturday, July 20, 2013 3:25 AM *To:* Alex Huang *Cc:* Hugo Trippaers; cloudstack *Subject:* Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs Alex, it falls through to 'UnDecided' if not known. Do you mean it should throw something? or maybe add an extra 'UnDefined'? On Fri, Jul 19, 2013 at 5:31 PM, Alex Huang alex.hu...@citrix.com wrote: This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12685/ Ship it! master: 2d4464d One review comment is if you are checking for the colon to see if the value has the schema already, then shouldn't it also check if the schema matches what's declared? But that shouldn't block this review. The code itself is technically sound. - Alex Huang On July 17th, 2013, 3:46 p.m. UTC, daan Hoogland wrote: Review request for cloudstack and Hugo Trippaers. By daan Hoogland. *Updated July 17, 2013, 3:46 p.m.* *Bugs: *CLOUDSTACK-1532 *Repository: *cloudstack-git Description the review for the complete patch was open to long and no longer applicable, so I am starting with smaller patches as this is really needed to implement CLOUDSTACK-1532 Testing unit testing Diffs - api/src/com/cloud/network/Networks.java (5aede05) - api/test/com/cloud/network/NetworksTest.java (PRE-CREATION) View Diff https://reviews.apache.org/r/12685/diff/ ** **
Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs
Alex, it falls through to 'UnDecided' if not known. Do you mean it should throw something? or maybe add an extra 'UnDefined'? On Fri, Jul 19, 2013 at 5:31 PM, Alex Huang alex.hu...@citrix.com wrote: This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12685/ Ship it! master: 2d4464d One review comment is if you are checking for the colon to see if the value has the schema already, then shouldn't it also check if the schema matches what's declared? But that shouldn't block this review. The code itself is technically sound. - Alex Huang On July 17th, 2013, 3:46 p.m. UTC, daan Hoogland wrote: Review request for cloudstack and Hugo Trippaers. By daan Hoogland. *Updated July 17, 2013, 3:46 p.m.* *Bugs: * CLOUDSTACK-1532 *Repository: * cloudstack-git Description the review for the complete patch was open to long and no longer applicable, so I am starting with smaller patches as this is really needed to implement CLOUDSTACK-1532 Testing unit testing Diffs - api/src/com/cloud/network/Networks.java (5aede05) - api/test/com/cloud/network/NetworksTest.java (PRE-CREATION) View Diff https://reviews.apache.org/r/12685/diff/
RE: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs
Daan, It should assert and throw a CloudRuntimeException. https://cwiki.apache.org/confluence/display/CLOUDSTACK/Exceptions+and+logging --Alex From: Daan Hoogland [mailto:daan.hoogl...@gmail.com] Sent: Saturday, July 20, 2013 3:25 AM To: Alex Huang Cc: Hugo Trippaers; cloudstack Subject: Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs Alex, it falls through to 'UnDecided' if not known. Do you mean it should throw something? or maybe add an extra 'UnDefined'? On Fri, Jul 19, 2013 at 5:31 PM, Alex Huang alex.hu...@citrix.commailto:alex.hu...@citrix.com wrote: This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12685/ Ship it! master: 2d4464d One review comment is if you are checking for the colon to see if the value has the schema already, then shouldn't it also check if the schema matches what's declared? But that shouldn't block this review. The code itself is technically sound. - Alex Huang On July 17th, 2013, 3:46 p.m. UTC, daan Hoogland wrote: Review request for cloudstack and Hugo Trippaers. By daan Hoogland. Updated July 17, 2013, 3:46 p.m. Bugs: CLOUDSTACK-1532 Repository: cloudstack-git Description the review for the complete patch was open to long and no longer applicable, so I am starting with smaller patches as this is really needed to implement CLOUDSTACK-1532 Testing unit testing Diffs * api/src/com/cloud/network/Networks.java (5aede05) * api/test/com/cloud/network/NetworksTest.java (PRE-CREATION) View Diffhttps://reviews.apache.org/r/12685/diff/
Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12685/#review23493 --- It seems good to me? Code-wise I can't find anything that isn't right. I see that Spark404 is also mentioned here, did he get a chance to look at this yet? - Wido den Hollander On July 17, 2013, 3:46 p.m., daan Hoogland wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12685/ --- (Updated July 17, 2013, 3:46 p.m.) Review request for cloudstack and Hugo Trippaers. Bugs: CLOUDSTACK-1532 Repository: cloudstack-git Description --- the review for the complete patch was open to long and no longer applicable, so I am starting with smaller patches as this is really needed to implement CLOUDSTACK-1532 Diffs - api/src/com/cloud/network/Networks.java 5aede05 api/test/com/cloud/network/NetworksTest.java PRE-CREATION Diff: https://reviews.apache.org/r/12685/diff/ Testing --- unit testing Thanks, daan Hoogland
Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs
On July 19, 2013, 9:41 a.m., Wido den Hollander wrote: It seems good to me? Code-wise I can't find anything that isn't right. I see that Spark404 is also mentioned here, did he get a chance to look at this yet? Yeah, i'm ok with this. I'm actually abusing this patch to test jenkins-reviewboard integration, thats why i haven't committed it. - Hugo --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12685/#review23493 --- On July 17, 2013, 3:46 p.m., daan Hoogland wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12685/ --- (Updated July 17, 2013, 3:46 p.m.) Review request for cloudstack and Hugo Trippaers. Bugs: CLOUDSTACK-1532 Repository: cloudstack-git Description --- the review for the complete patch was open to long and no longer applicable, so I am starting with smaller patches as this is really needed to implement CLOUDSTACK-1532 Diffs - api/src/com/cloud/network/Networks.java 5aede05 api/test/com/cloud/network/NetworksTest.java PRE-CREATION Diff: https://reviews.apache.org/r/12685/diff/ Testing --- unit testing Thanks, daan Hoogland
Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12685/#review23515 --- Ship it! master: 2d4464d One review comment is if you are checking for the colon to see if the value has the schema already, then shouldn't it also check if the schema matches what's declared? But that shouldn't block this review. The code itself is technically sound. - Alex Huang On July 17, 2013, 3:46 p.m., daan Hoogland wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12685/ --- (Updated July 17, 2013, 3:46 p.m.) Review request for cloudstack and Hugo Trippaers. Bugs: CLOUDSTACK-1532 Repository: cloudstack-git Description --- the review for the complete patch was open to long and no longer applicable, so I am starting with smaller patches as this is really needed to implement CLOUDSTACK-1532 Diffs - api/src/com/cloud/network/Networks.java 5aede05 api/test/com/cloud/network/NetworksTest.java PRE-CREATION Diff: https://reviews.apache.org/r/12685/diff/ Testing --- unit testing Thanks, daan Hoogland
