You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by chengxinglin <gi...@git.apache.org> on 2017/05/27 09:07:43 UTC

[GitHub] storm pull request #2142: MINOR: Fix pacemaker_state_factory.clj not compile...

GitHub user chengxinglin opened a pull request:

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

    MINOR: Fix pacemaker_state_factory.clj not compile problem

    Fix pacemaker_state_factory.clj not compile problem

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

    $ git pull https://github.com/chengxinglin/storm pacemaker_state_factory_no_compile

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

    https://github.com/apache/storm/pull/2142.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 #2142
    
----
commit cf89fa631f36ae9c98a1ed5e3a64a8437232fd67
Author: unknown <17...@sn.suning.ad>
Date:   2017-05-27T09:05:13Z

    MINOR: Fix pacemaker_state_factory.clj not compile 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 #2142: MINOR: Fix pacemaker_state_factory.clj not compile...

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

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


---

[GitHub] storm pull request #2142: MINOR: Fix pacemaker_state_factory.clj not compile...

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

    https://github.com/apache/storm/pull/2142#discussion_r119911355
  
    --- Diff: storm-core/src/clj/org/apache/storm/pacemaker/pacemaker_state_factory.clj ---
    @@ -106,16 +106,15 @@
     (defn get-pacemaker-write-client [conf servers client-pool]
       ;; Client should be created in case of an exception or first write call
       ;; Shutdown happens in the retry loop
    -  (try 
    -    (.waitUntilReady
    -     (let [client (get @client-pool (first @servers))]
    -       (if (nil? client)
    -         (do
    -           (swap! client-pool merge {(first @servers) (PacemakerClient. conf (first @servers))})
    -           (get @client-pool (first @servers)))
    -         client)))
    -    (catch Exception e
    -      (throw e))))
    +  (let [client (get @client-pool (first @servers))]
    +    (try
    +        (.waitUntilReady
    +            (let [] (if (nil? client)
    +                 (do (swap! client-pool merge {(first @servers) (PacemakerClient. conf (first @servers))})
    +                     (get @client-pool (first @servers)))
    +                      client)))
    +       (catch Exception e (throw e)))
    +    (get @client-pool (first @servers))))
    --- End diff --
    
    waitUntilReady is not actually needed because send will call it internally.  We can just skip calling it here all together.  This then lets us have something like ...
    
    ```
    (defn get-pacemaker-write-client [conf servers client-pool]
       ;; Client should be created in case of an exception or first write call
       ;; Shutdown happens in the retry loop
       (let [server (first @servers)]
              (or (get @client-pool server)
                  (get (swap! client-pool merge {server (PacemakerClient. conf server)}) server))))
    ```
    
    and we can then remove the update to PacemakerClient.java


---
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 issue #2142: MINOR: Fix pacemaker_state_factory.clj not compile proble...

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

    https://github.com/apache/storm/pull/2142
  
    @HeartSaVioR yes.


---
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 issue #2142: MINOR: Fix pacemaker_state_factory.clj not compile proble...

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:

    https://github.com/apache/storm/pull/2142
  
    @chengxinglin Kindly reminder.
    @revans2 If @chengxinglin doesn't respond and if you think this is a blocker, could you craft pull request after waiting a bit?


---
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 issue #2142: MINOR: Fix pacemaker_state_factory.clj not compile proble...

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:

    https://github.com/apache/storm/pull/2142
  
    @chengxinglin Do you have any updates?


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