[GitHub] rhtyd commented on issue #2273: CLOUDSTACK-10090:createPortForwardingRule api call accepts 'halt' as ?
rhtyd commented on issue #2273: CLOUDSTACK-10090:createPortForwardingRule api call accepts 'halt' as ? URL: https://github.com/apache/cloudstack/pull/2273#issuecomment-342710794 Tests LGTM, given the provided feedback and explainations I'll merge this now. Thanks @mrunalinikankariya and @jayapalu This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on issue #2273: CLOUDSTACK-10090:createPortForwardingRule api call accepts 'halt' as ?
rhtyd commented on issue #2273: CLOUDSTACK-10090:createPortForwardingRule api call accepts 'halt' as ? URL: https://github.com/apache/cloudstack/pull/2273#issuecomment-342710794 Tests LGTM, given the provided feedback and explanations I'll merge this now. Thanks @mrunalinikankariya and @jayapalu This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on issue #2273: CLOUDSTACK-10090:createPortForwardingRule api call accepts 'halt' as ?
rhtyd commented on issue #2273: CLOUDSTACK-10090:createPortForwardingRule api call accepts 'halt' as ? URL: https://github.com/apache/cloudstack/pull/2273#issuecomment-342382960 Okay @jayapalu rekicking tests. @blueorangutan test This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on issue #2273: CLOUDSTACK-10090:createPortForwardingRule api call accepts 'halt' as ?
rhtyd commented on issue #2273: CLOUDSTACK-10090:createPortForwardingRule api call accepts 'halt' as ? URL: https://github.com/apache/cloudstack/pull/2273#issuecomment-342371413 @jayapalu @mrunalinikankariya looks like there are several regression failures, this cannot be accepted as such, you can consider refactoring the logic to throw exception only for unsupported keywords/protocols This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on issue #2273: CLOUDSTACK-10090:createPortForwardingRule api call accepts 'halt' as ?
rhtyd commented on issue #2273: CLOUDSTACK-10090:createPortForwardingRule api call accepts 'halt' as ? URL: https://github.com/apache/cloudstack/pull/2273#issuecomment-342371413 @jayapalu @mrunalinikankariya looks like there are several regression failures, this cannot be accepted as such, please refactor the logic to throw exception only for unsupported keywords/protocols This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on issue #2273: CLOUDSTACK-10090:createPortForwardingRule api call accepts 'halt' as ?
rhtyd commented on issue #2273: CLOUDSTACK-10090:createPortForwardingRule api call accepts 'halt' as ? URL: https://github.com/apache/cloudstack/pull/2273#issuecomment-342143367 Thanks @mrunalinikankariya for the explanation, however, I'm wondering if this might break use-cases where you would want to port-forward non-tcp/non-udp traffic such as `esp`, `ah`. Instead, can we add list of keywords that should not be allowed to be added to the protocol option? /cc @PaulAngus @DaanHoogland @nvazquez @borisstoyanov @wido @swill and others This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on issue #2273: CLOUDSTACK-10090:createPortForwardingRule api call accepts 'halt' as ?
rhtyd commented on issue #2273: CLOUDSTACK-10090:createPortForwardingRule api call accepts 'halt' as ? URL: https://github.com/apache/cloudstack/pull/2273#issuecomment-342088344 @blueorangutan test This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on issue #2273: CLOUDSTACK-10090:createPortForwardingRule api call accepts 'halt' as ?
rhtyd commented on issue #2273: CLOUDSTACK-10090:createPortForwardingRule api call accepts 'halt' as ? URL: https://github.com/apache/cloudstack/pull/2273#issuecomment-342044537 @mrunalinikankariya can you explain where/how this method is used and if putting a hardcoded protocol list can have undesirable effects, for example what if not both `tcp,udp` don't need to be port-forwarded. For merging, this needs to be tested first. @blueorangutan package This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
