You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by redsanket <gi...@git.apache.org> on 2015/07/22 21:50:54 UTC

[GitHub] storm pull request: [STORM-440] completed exposing the drpcclient ...

GitHub user redsanket opened a pull request:

    https://github.com/apache/storm/pull/648

    [STORM-440] completed exposing the drpcclient and nimbusclient without need for configs

    completed exposing the drpcclient and nimbusclient without the need for config initialization

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

    $ git pull https://github.com/redsanket/storm STORM-440

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

    https://github.com/apache/storm/pull/648.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 #648
    
----
commit 5045f26d10c4cff782cf1d8661d0b192a4a5332d
Author: Sanket <sc...@untilservice-lm>
Date:   2015-07-22T19:42:59Z

    completed exposing the drpcclient and nimbusclient without the need for config initialization

commit 207d9a671fd4d7aa90191d227a10cded10c555af
Author: Sanket <sc...@untilservice-lm>
Date:   2015-07-22T19:48:37Z

    indentation edits

commit 76f0c4c3f3f9e2d57de8e0a735962d2ec7f22bb2
Author: Sanket <sc...@untilservice-lm>
Date:   2015-07-22T19:51:02Z

    removed unneccessary constructor created while solving the problem

----


---
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] storm pull request: [STORM-440] completed exposing the drpcclient ...

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

    https://github.com/apache/storm/pull/648#discussion_r47552355
  
    --- Diff: storm-core/src/jvm/backtype/storm/utils/NimbusClient.java ---
    @@ -30,11 +30,18 @@
         private Nimbus.Client _client;
         private static final Logger LOG = LoggerFactory.getLogger(NimbusClient.class);
     
    +    public static NimbusClient getConfiguredClient() {
    +      return getConfiguredClient(null);
    +    }
     
         public static NimbusClient getConfiguredClient(Map conf) {
    -        try {
    -            String nimbusHost = (String) conf.get(Config.NIMBUS_HOST);
    -            return new NimbusClient(conf, nimbusHost);
    +      try {
    +        Map fullConf = Utils.readStormConfig();
    +        if (conf != null) {
    +         fullConf.putAll(conf);
    +        }
    +        String nimbusHost = (String) fullConf.get(Config.NIMBUS_HOST);
    +              return new NimbusClient(fullConf, nimbusHost);
    --- End diff --
    
    very minor, spacing issue.


---
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] storm pull request: [STORM-440] completed exposing the drpcclient ...

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:

    https://github.com/apache/storm/pull/648#issuecomment-164548714
  
    +1 needs to be upmerged though.


---
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] storm pull request: [STORM-440] completed exposing the drpcclient ...

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

    https://github.com/apache/storm/pull/648#discussion_r35406122
  
    --- Diff: storm-core/src/jvm/backtype/storm/utils/DRPCClient.java ---
    @@ -46,7 +46,22 @@ public DRPCClient(Map conf, String host, int port, Integer timeout) throws TTran
             this.port = port;
             this.client = new DistributedRPC.Client(_protocol);
         }
    -        
    +
    +    public static DistributedRPC.Client getConfiguredDRPCClient(String host, int port) {
    +    return getConfiguredDRPCClient(null, host, port);
    +    }
    +
    +    public static DistributedRPC.Client getConfiguredDRPCClient(Map conf, String host, int port) {
    +      try {
    +        Map fullConf = Utils.readStormConfig();
    +        if (conf != null) {
    +          fullConf.putAll(conf);
    +        }
    +        return new DRPCClient(conf, host, port).getClient();
    --- End diff --
    
    shoud be ```return new DRPCClient(fullConf, host, port).getClient();```


---
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] storm pull request: [STORM-440] completed exposing the drpcclient ...

Posted by ppoulosk <gi...@git.apache.org>.
Github user ppoulosk commented on the pull request:

    https://github.com/apache/storm/pull/648#issuecomment-164546402
  
    +1 NB


---
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] storm pull request: [STORM-440] completed exposing the drpcclient ...

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

    https://github.com/apache/storm/pull/648#discussion_r35406230
  
    --- Diff: storm-core/src/jvm/backtype/storm/utils/NimbusClient.java ---
    @@ -30,11 +30,18 @@
         private Nimbus.Client _client;
         private static final Logger LOG = LoggerFactory.getLogger(NimbusClient.class);
     
    +    public static NimbusClient getConfiguredClient() {
    +      return getConfiguredClient(null);
    +    }
     
         public static NimbusClient getConfiguredClient(Map conf) {
    -        try {
    -            String nimbusHost = (String) conf.get(Config.NIMBUS_HOST);
    -            return new NimbusClient(conf, nimbusHost);
    +      try {
    +        Map fullConf = Utils.readStormConfig();
    +        if (conf != null) {
    +         fullConf.putAll(conf);
    +        }
    +        String nimbusHost = (String) conf.get(Config.NIMBUS_HOST);
    +              return new NimbusClient(conf, nimbusHost);
    --- End diff --
    
    should be 
    ```
     String nimbusHost = (String) fullConf.get(Config.NIMBUS_HOST);
     return new NimbusClient(fullConf, nimbusHost);
    ```


---
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] storm pull request: [STORM-440] completed exposing the drpcclient without ne...

Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the pull request:

    https://github.com/apache/storm/pull/648
  
    +1


---
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] storm pull request: [STORM-440] completed exposing the drpcclient ...

Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the pull request:

    https://github.com/apache/storm/pull/648#issuecomment-195885039
  
    @redsanket can you upmerge and squash those commits.


---
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] storm pull request #648: [STORM-440] completed exposing the drpcclient witho...

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

    https://github.com/apache/storm/pull/648


---

[GitHub] storm pull request: [STORM-440] completed exposing the drpcclient and nimbus...

Posted by redsanket <gi...@git.apache.org>.
Github user redsanket commented on the pull request:

    https://github.com/apache/storm/pull/648
  
    @harshach updated and squashed thank you


---
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] storm pull request: [STORM-440] completed exposing the drpcclient ...

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

    https://github.com/apache/storm/pull/648#discussion_r35439293
  
    --- Diff: storm-core/src/jvm/backtype/storm/utils/NimbusClient.java ---
    @@ -30,11 +30,18 @@
         private Nimbus.Client _client;
         private static final Logger LOG = LoggerFactory.getLogger(NimbusClient.class);
     
    +    public static NimbusClient getConfiguredClient() {
    +      return getConfiguredClient(null);
    +    }
     
         public static NimbusClient getConfiguredClient(Map conf) {
    -        try {
    -            String nimbusHost = (String) conf.get(Config.NIMBUS_HOST);
    -            return new NimbusClient(conf, nimbusHost);
    +      try {
    +        Map fullConf = Utils.readStormConfig();
    +        if (conf != null) {
    +         fullConf.putAll(conf);
    +        }
    +        String nimbusHost = (String) conf.get(Config.NIMBUS_HOST);
    +              return new NimbusClient(conf, nimbusHost);
    --- End diff --
    
    Sorry pushed the wrong commit, shall do it now, thanks for pointing it out


---
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.
---