You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by brymaven <gi...@git.apache.org> on 2016/03/26 01:39:25 UTC

[GitHub] storm pull request: STORM-515: Clojure documentation and examples

GitHub user brymaven opened a pull request:

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

    STORM-515: Clojure documentation and examples

    Add some more examples with Clojure, which show using parameters with config and global grouping.
    Also add tests for all clojure bolts.

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

    $ git pull https://github.com/brymaven/storm STORM-515

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

    https://github.com/apache/storm/pull/1262.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 #1262
    
----
commit 8c31e4ddf9012eaf13d76e6c16684037b31fe5ef
Author: Bryant <br...@gmail.com>
Date:   2016-03-21T00:15:12Z

    Correct typo on example javadoc

commit 843d6904369ab7db584bcef6e9a62e6be2845b7a
Author: Bryant <br...@gmail.com>
Date:   2016-03-24T20:37:20Z

    Fix indent on clojure testing function

commit 3b7c4a81d7c5111eb6d506bef7cedf9254bf91d0
Author: Bryant <br...@gmail.com>
Date:   2016-03-24T20:42:45Z

    Add Clojure examples for rolling top words and exclamation

----


---
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-515: Clojure documentation and examples

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

    https://github.com/apache/storm/pull/1262#issuecomment-204430280
  
    +1
    Nice.


---
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-515: Clojure documentation and examples

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

    https://github.com/apache/storm/pull/1262#discussion_r58132657
  
    --- Diff: examples/storm-starter/test/clj/org/apache/storm/starter/clj/bolts_test.clj ---
    @@ -0,0 +1,115 @@
    +;; Licensed to the Apache Software Foundation (ASF) under one
    +;; or more contributor license agreements.  See the NOTICE file
    +;; distributed with this work for additional information
    +;; regarding copyright ownership.  The ASF licenses this file
    +;; to you under the Apache License, Version 2.0 (the
    +;; "License"); you may not use this file except in compliance
    +;; with the License.  You may obtain a copy of the License at
    +;;
    +;; http://www.apache.org/licenses/LICENSE-2.0
    +;;
    +;; Unless required by applicable law or agreed to in writing, software
    +;; distributed under the License is distributed on an "AS IS" BASIS,
    +;; WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +;; See the License for the specific language governing permissions and
    +;; limitations under the License.
    +(ns org.apache.storm.starter.clj.bolts-test
    +  (:require [clojure.test :refer :all]
    +            [org.apache.storm.starter.clj.word-count :refer [word-count split-sentence]]
    +            [org.apache.storm.starter.clj.exclamation :refer [exclamation-bolt]]
    +            [org.apache.storm.starter.clj.bolts :refer
    +             [rolling-count-bolt intermediate-rankings-bolt total-rankings-bolt]]
    +            [org.apache.storm [testing :refer :all]])
    +  (:import [org.apache.storm Constants]
    +           [org.apache.storm.task OutputCollector IOutputCollector]
    +           [org.apache.storm.starter.tools Rankable]
    +           [org.apache.storm.tuple Tuple]))
    +
    +(defmacro test-with-tuple [[bolt tuples] & body]
    +  `(let [bolt# ~bolt
    +         tuples# (atom [])]
    +     (.prepare bolt# {} nil (OutputCollector.
    +                       (reify IOutputCollector
    +                         (emit [_ _ _ tuple#]
    +                           (swap! tuples# conj tuple#))
    +                         (ack [_ input]))))
    +     (if (vector? ~tuples)
    +       (doseq [t# ~tuples]
    +         (.execute bolt# t#))
    +       (.execute bolt# ~tuples))
    +     (let [~'tuples @tuples#]
    --- End diff --
    
    `~'tuples` evaluates to `tuples` This makes the macro unhygienic.


---
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-515: Clojure documentation and examples

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

    https://github.com/apache/storm/pull/1262#issuecomment-204238722
  
    Fixed.


---
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-515: Clojure documentation and examples

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

    https://github.com/apache/storm/pull/1262#issuecomment-204199193
  
    Good suggestions. I fixed the macros.


---
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-515: Clojure documentation and examples

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

    https://github.com/apache/storm/pull/1262#discussion_r58153392
  
    --- Diff: examples/storm-starter/src/clj/org/apache/storm/starter/clj/bolts.clj ---
    @@ -0,0 +1,78 @@
    +;; Licensed to the Apache Software Foundation (ASF) under one
    +;; or more contributor license agreements.  See the NOTICE file
    +;; distributed with this work for additional information
    +;; regarding copyright ownership.  The ASF licenses this file
    +;; to you under the Apache License, Version 2.0 (the
    +;; "License"); you may not use this file except in compliance
    +;; with the License.  You may obtain a copy of the License at
    +;;
    +;; http://www.apache.org/licenses/LICENSE-2.0
    +;;
    +;; Unless required by applicable law or agreed to in writing, software
    +;; distributed under the License is distributed on an "AS IS" BASIS,
    +;; WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +;; See the License for the specific language governing permissions and
    +;; limitations under the License.
    +(ns org.apache.storm.starter.clj.bolts
    +  (:require [org.apache.storm
    +             [clojure :refer :all]
    +             [config :refer :all]
    +             [log :refer :all]])
    +  (:import [org.apache.storm.starter.tools
    +            NthLastModifiedTimeTracker SlidingWindowCounter
    +            Rankings RankableObjectWithFields]
    +           [org.apache.storm.utils TupleUtils]))
    +
    +(defbolt rolling-count-bolt ["obj" "count" "actualWindowLengthInSeconds"]
    +  {:prepare true
    +   :params [window-length emit-frequency]
    +   :conf {TOPOLOGY-TICK-TUPLE-FREQ-SECS emit-frequency}}
    +  [conf context collector]
    +  (let [num-windows (/ window-length emit-frequency)
    +        counter (SlidingWindowCounter. num-windows)
    +        tracker (NthLastModifiedTimeTracker. num-windows)]
    +    (bolt
    +     (execute [{word "word" :as tuple}]
    +       (if (TupleUtils/isTick tuple)
    +         (let [counts (.getCountsThenAdvanceWindow counter)
    +               actual-window-length (.secondsSinceOldestModification tracker)]
    +           (log-debug "Received tick tuple, triggering emit of current window counts")
    +           (.markAsModified tracker)
    +           (doseq [[obj count] counts]
    +             (emit-bolt! collector [obj count actual-window-length])))
    +         (do
    +           (.incrementCount counter word)
    +           (ack! collector tuple)))))))
    +
    +(defmacro update-rankings [tuple & body]
    +  `(if (TupleUtils/isTick ~tuple)
    +     (do
    +       (log-debug "Received tick tuple, triggering emit of current rankings")
    +       (emit-bolt! ~'collector [(.copy ~'rankings)])
    --- End diff --
    
    My previous comment applies to `collector` and `rankings` as well


---
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-515: Clojure documentation and examples

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

    https://github.com/apache/storm/pull/1262#discussion_r58131243
  
    --- Diff: examples/storm-starter/src/clj/org/apache/storm/starter/clj/bolts.clj ---
    @@ -0,0 +1,75 @@
    +;; Licensed to the Apache Software Foundation (ASF) under one
    +;; or more contributor license agreements.  See the NOTICE file
    +;; distributed with this work for additional information
    +;; regarding copyright ownership.  The ASF licenses this file
    +;; to you under the Apache License, Version 2.0 (the
    +;; "License"); you may not use this file except in compliance
    +;; with the License.  You may obtain a copy of the License at
    +;;
    +;; http://www.apache.org/licenses/LICENSE-2.0
    +;;
    +;; Unless required by applicable law or agreed to in writing, software
    +;; distributed under the License is distributed on an "AS IS" BASIS,
    +;; WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +;; See the License for the specific language governing permissions and
    +;; limitations under the License.
    +(ns org.apache.storm.starter.clj.bolts
    +  (:require [org.apache.storm
    +             [clojure :refer :all]
    +             [config :refer :all]
    +             [log :refer :all]])
    +  (:import [org.apache.storm.starter.tools
    +            NthLastModifiedTimeTracker SlidingWindowCounter
    +            Rankings RankableObjectWithFields]
    +           [org.apache.storm.utils TupleUtils]))
    +
    +(defbolt rolling-count-bolt ["obj" "count" "actualWindowLengthInSeconds"]
    +  {:prepare true
    +   :params [window-length emit-frequency]
    +   :conf {TOPOLOGY-TICK-TUPLE-FREQ-SECS emit-frequency}}
    +  [conf context collector]
    +  (let [num-windows (/ window-length emit-frequency)
    +        counter (SlidingWindowCounter. num-windows)
    +        tracker (NthLastModifiedTimeTracker. num-windows)]
    +    (bolt
    +     (execute [{word "word" :as tuple}]
    +       (if (TupleUtils/isTick tuple)
    +         (let [counts (.getCountsThenAdvanceWindow counter)
    +               actual-window-length (.secondsSinceOldestModification tracker)]
    +           (log-debug "Received tick tuple, triggering emit of current window counts")
    +           (.markAsModified tracker)
    +           (doseq [[obj count] counts]
    +             (emit-bolt! collector [obj count actual-window-length])))
    +         (do
    +           (.incrementCount counter word)
    +           (ack! collector tuple)))))))
    +
    +(defmacro update-rankings [& body]
    +  `(if (TupleUtils/isTick ~'tuple)
    --- End diff --
    
    Relying on symbols from the expansion environment is sort of a bad idea. It does make the call look cleaner, but it leaves potential for accidental breakage in mysterious ways.



---
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-515: Clojure documentation and examples

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

    https://github.com/apache/storm/pull/1262#issuecomment-204148932
  
    Looks good for the most part.


---
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-515: Clojure documentation and examples

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

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


---
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-515: Clojure documentation and examples

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

    https://github.com/apache/storm/pull/1262#issuecomment-204124801
  
    +1 looks good to me.


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