You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stratos.apache.org by anuruddhal <gi...@git.apache.org> on 2015/08/12 19:19:44 UTC

[GitHub] stratos pull request: Adding PORT_MAPPINGS to payload parameters i...

GitHub user anuruddhal opened a pull request:

    https://github.com/apache/stratos/pull/432

    Adding PORT_MAPPINGS to payload parameters in jcloud IaaS

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/anuruddhal/stratos master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/stratos/pull/432.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #432
    
----
commit 787aa8fc136b3a153780d14d8fa84ca5bce850ed
Author: anuruddhal <an...@gmail.com>
Date:   2015-08-12T17:17:32Z

    Adding PORT_MAPPINGS to payload parameters in jcloud IaaS

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] stratos pull request: Adding PORT_MAPPINGS to payload parameters i...

Posted by imesh <gi...@git.apache.org>.
Github user imesh commented on a diff in the pull request:

    https://github.com/apache/stratos/pull/432#discussion_r36938566
  
    --- Diff: components/org.apache.stratos.autoscaler/src/main/java/org/apache/stratos/autoscaler/applications/ApplicationUtils.java ---
    @@ -88,8 +88,7 @@ private static String createPortMappingPayloadString(Cartridge cartridge) {
             }
     
             for (PortMapping portMapping : portMappings) {
    -            int port = portMapping.getPort();
    -            portMapBuilder.append(port).append("|");
    +            portMapBuilder.append(portMapping.getPort()).append("|");
    --- End diff --
    
    Shall we move character '|' to a constant?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] stratos pull request: Adding PORT_MAPPINGS to payload parameters i...

Posted by imesh <gi...@git.apache.org>.
Github user imesh commented on a diff in the pull request:

    https://github.com/apache/stratos/pull/432#discussion_r36938529
  
    --- Diff: components/org.apache.stratos.autoscaler/src/main/java/org/apache/stratos/autoscaler/applications/ApplicationUtils.java ---
    @@ -76,7 +76,7 @@ public static Properties getGlobalPayloadData() {
             return globalProperties;
         }
     
    -    private static String createPortMappingPayloadString(Cartridge cartridge) {
    +    private static String createPortsToPayloadString(Cartridge cartridge) {
    --- End diff --
    
    It would be better to add method comments.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] stratos pull request: Adding PORT_MAPPINGS to payload parameters i...

Posted by anuruddhal <gi...@git.apache.org>.
Github user anuruddhal commented on a diff in the pull request:

    https://github.com/apache/stratos/pull/432#discussion_r36939427
  
    --- Diff: components/org.apache.stratos.autoscaler/src/main/java/org/apache/stratos/autoscaler/applications/ApplicationUtils.java ---
    @@ -76,7 +76,7 @@ public static Properties getGlobalPayloadData() {
             return globalProperties;
         }
     
    -    private static String createPortMappingPayloadString(Cartridge cartridge) {
    +    private static String createPortsToPayloadString(Cartridge cartridge) {
    --- End diff --
    
    Added method comment


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] stratos pull request: Adding PORT_MAPPINGS to payload parameters i...

Posted by anuruddhal <gi...@git.apache.org>.
Github user anuruddhal commented on a diff in the pull request:

    https://github.com/apache/stratos/pull/432#discussion_r36939478
  
    --- Diff: components/org.apache.stratos.autoscaler/src/main/java/org/apache/stratos/autoscaler/applications/ApplicationUtils.java ---
    @@ -98,6 +97,31 @@ private static String createPortMappingPayloadString(Cartridge cartridge) {
             return portMappingString;
         }
     
    +    private static String createPortMappingsToPayloadString(Cartridge cartridge) {
    +
    +        // port mappings
    +        StringBuilder portMapBuilder = new StringBuilder();
    +        PortMapping[] portMappings = cartridge.getPortMappings();
    +
    +        if (cartridge.getPortMappings()[0] == null) {
    +            // first element is null, which means no port mappings.
    +            return null;
    +        }
    +
    +        for (PortMapping portMapping : portMappings) {
    +            int port = portMapping.getPort();
    +            //'NAME:mgt-console|PROTOCOL:https|PORT:30649|PROXY_PORT:0|TYPE:NodePort
    +            portMapBuilder.append(String.format("NAME:%s|PROTOCOL:%s|PORT:%d|PROXY_PORT:%d|TYPE:%s;",
    +                    portMapping.getName(), portMapping.getProtocol(),
    +                    portMapping.getPort(), portMapping.getProxyPort(),
    +                    portMapping.getKubernetesPortType()));
    +        }
    +        //remove last ;
    +        String portMappingString = portMapBuilder.toString().replaceAll(";$", "");
    +        return portMappingString;
    --- End diff --
    
    This is the the regex to remove last ";" character from the string


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] stratos pull request: Adding PORT_MAPPINGS to payload parameters i...

Posted by imesh <gi...@git.apache.org>.
Github user imesh commented on a diff in the pull request:

    https://github.com/apache/stratos/pull/432#discussion_r36938605
  
    --- Diff: components/org.apache.stratos.autoscaler/src/main/java/org/apache/stratos/autoscaler/applications/ApplicationUtils.java ---
    @@ -98,6 +97,31 @@ private static String createPortMappingPayloadString(Cartridge cartridge) {
             return portMappingString;
         }
     
    +    private static String createPortMappingsToPayloadString(Cartridge cartridge) {
    +
    +        // port mappings
    +        StringBuilder portMapBuilder = new StringBuilder();
    +        PortMapping[] portMappings = cartridge.getPortMappings();
    +
    +        if (cartridge.getPortMappings()[0] == null) {
    +            // first element is null, which means no port mappings.
    +            return null;
    +        }
    +
    +        for (PortMapping portMapping : portMappings) {
    +            int port = portMapping.getPort();
    +            //'NAME:mgt-console|PROTOCOL:https|PORT:30649|PROXY_PORT:0|TYPE:NodePort
    +            portMapBuilder.append(String.format("NAME:%s|PROTOCOL:%s|PORT:%d|PROXY_PORT:%d|TYPE:%s;",
    +                    portMapping.getName(), portMapping.getProtocol(),
    +                    portMapping.getPort(), portMapping.getProxyPort(),
    +                    portMapping.getKubernetesPortType()));
    +        }
    +        //remove last ;
    --- End diff --
    
    Why do we have a semi-colon at the end of this comment?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] stratos pull request: Adding PORT_MAPPINGS to payload parameters i...

Posted by imesh <gi...@git.apache.org>.
Github user imesh commented on a diff in the pull request:

    https://github.com/apache/stratos/pull/432#discussion_r36938573
  
    --- Diff: components/org.apache.stratos.autoscaler/src/main/java/org/apache/stratos/autoscaler/applications/ApplicationUtils.java ---
    @@ -98,6 +97,31 @@ private static String createPortMappingPayloadString(Cartridge cartridge) {
             return portMappingString;
         }
     
    +    private static String createPortMappingsToPayloadString(Cartridge cartridge) {
    --- End diff --
    
    Please add method comments.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] stratos pull request: Adding PORT_MAPPINGS to payload parameters i...

Posted by anuruddhal <gi...@git.apache.org>.
Github user anuruddhal commented on a diff in the pull request:

    https://github.com/apache/stratos/pull/432#discussion_r36939440
  
    --- Diff: components/org.apache.stratos.autoscaler/src/main/java/org/apache/stratos/autoscaler/applications/ApplicationUtils.java ---
    @@ -88,8 +88,7 @@ private static String createPortMappingPayloadString(Cartridge cartridge) {
             }
     
             for (PortMapping portMapping : portMappings) {
    -            int port = portMapping.getPort();
    -            portMapBuilder.append(port).append("|");
    +            portMapBuilder.append(portMapping.getPort()).append("|");
    --- End diff --
    
    Moved to a constant


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] stratos pull request: Adding PORT_MAPPINGS to payload parameters i...

Posted by imesh <gi...@git.apache.org>.
Github user imesh commented on a diff in the pull request:

    https://github.com/apache/stratos/pull/432#discussion_r36938635
  
    --- Diff: components/org.apache.stratos.autoscaler/src/main/java/org/apache/stratos/autoscaler/applications/ApplicationUtils.java ---
    @@ -98,6 +97,31 @@ private static String createPortMappingPayloadString(Cartridge cartridge) {
             return portMappingString;
         }
     
    +    private static String createPortMappingsToPayloadString(Cartridge cartridge) {
    +
    +        // port mappings
    +        StringBuilder portMapBuilder = new StringBuilder();
    +        PortMapping[] portMappings = cartridge.getPortMappings();
    +
    +        if (cartridge.getPortMappings()[0] == null) {
    +            // first element is null, which means no port mappings.
    +            return null;
    +        }
    +
    +        for (PortMapping portMapping : portMappings) {
    +            int port = portMapping.getPort();
    +            //'NAME:mgt-console|PROTOCOL:https|PORT:30649|PROXY_PORT:0|TYPE:NodePort
    +            portMapBuilder.append(String.format("NAME:%s|PROTOCOL:%s|PORT:%d|PROXY_PORT:%d|TYPE:%s;",
    +                    portMapping.getName(), portMapping.getProtocol(),
    +                    portMapping.getPort(), portMapping.getProxyPort(),
    +                    portMapping.getKubernetesPortType()));
    +        }
    +        //remove last ;
    +        String portMappingString = portMapBuilder.toString().replaceAll(";$", "");
    +        return portMappingString;
    --- End diff --
    
    What's the purpose for this replace all command? From where do we get ";$"?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] stratos pull request: Adding PORT_MAPPINGS to payload parameters i...

Posted by anuruddhal <gi...@git.apache.org>.
Github user anuruddhal commented on a diff in the pull request:

    https://github.com/apache/stratos/pull/432#discussion_r36939445
  
    --- Diff: components/org.apache.stratos.autoscaler/src/main/java/org/apache/stratos/autoscaler/applications/ApplicationUtils.java ---
    @@ -98,6 +97,31 @@ private static String createPortMappingPayloadString(Cartridge cartridge) {
             return portMappingString;
         }
     
    +    private static String createPortMappingsToPayloadString(Cartridge cartridge) {
    --- End diff --
    
    Added method comments



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] stratos pull request: Adding PORT_MAPPINGS to payload parameters i...

Posted by anuruddhal <gi...@git.apache.org>.
Github user anuruddhal commented on a diff in the pull request:

    https://github.com/apache/stratos/pull/432#discussion_r36939461
  
    --- Diff: components/org.apache.stratos.autoscaler/src/main/java/org/apache/stratos/autoscaler/applications/ApplicationUtils.java ---
    @@ -98,6 +97,31 @@ private static String createPortMappingPayloadString(Cartridge cartridge) {
             return portMappingString;
         }
     
    +    private static String createPortMappingsToPayloadString(Cartridge cartridge) {
    +
    +        // port mappings
    +        StringBuilder portMapBuilder = new StringBuilder();
    +        PortMapping[] portMappings = cartridge.getPortMappings();
    +
    +        if (cartridge.getPortMappings()[0] == null) {
    +            // first element is null, which means no port mappings.
    +            return null;
    +        }
    +
    +        for (PortMapping portMapping : portMappings) {
    +            int port = portMapping.getPort();
    +            //'NAME:mgt-console|PROTOCOL:https|PORT:30649|PROXY_PORT:0|TYPE:NodePort
    +            portMapBuilder.append(String.format("NAME:%s|PROTOCOL:%s|PORT:%d|PROXY_PORT:%d|TYPE:%s;",
    +                    portMapping.getName(), portMapping.getProtocol(),
    +                    portMapping.getPort(), portMapping.getProxyPort(),
    +                    portMapping.getKubernetesPortType()));
    +        }
    +        //remove last ;
    --- End diff --
    
    Changed the comment


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] stratos pull request: Adding PORT_MAPPINGS to payload parameters i...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/stratos/pull/432


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---