Github user d2r commented on a diff in the pull request:

    https://github.com/apache/storm/pull/845#discussion_r45266348
  
    --- Diff: storm-core/src/clj/backtype/storm/daemon/supervisor.clj ---
    @@ -927,31 +1133,32 @@
            first ))
     
     (defmethod download-storm-code
    -    :local [conf storm-id master-code-dir supervisor download-lock]
    -    (let [stormroot (supervisor-stormdist-root conf storm-id)]
    -      (locking download-lock
    -            (FileUtils/copyDirectory (File. master-code-dir) (File. 
stormroot))
    -            (let [classloader (.getContextClassLoader 
(Thread/currentThread))
    -                  resources-jar (resources-jar)
    -                  url (.getResource classloader RESOURCES-SUBDIR)
    -                  target-dir (str stormroot file-path-separator 
RESOURCES-SUBDIR)]
    -              (cond
    -               resources-jar
    -               (do
    -                 (log-message "Extracting resources from jar at " 
resources-jar " to " target-dir)
    -                 (extract-dir-from-jar resources-jar RESOURCES-SUBDIR 
stormroot))
    -               url
    -               (do
    -                 (log-message "Copying resources at " (URI. (str url)) " 
to " target-dir)
    -                 (if (= (.getProtocol url) "jar" )
    -                   (extract-dir-from-jar (.getFile (.getJarFileURL 
(.openConnection url))) RESOURCES-SUBDIR stormroot)
    -                   (FileUtils/copyDirectory (File. (.getPath (URI. (str 
url)))) (File. target-dir)))
    -                 )
    -               )
    -              )
    -            )))
    -
    -(defmethod mk-code-distributor :local [conf] nil)
    +  :local [conf storm-id master-code-dir localizer]
    +  (let [tmproot (str (supervisor-tmp-dir conf) file-path-separator (uuid))
    +        stormroot (supervisor-stormdist-root conf storm-id)
    +        blob-store (Utils/getNimbusBlobStore conf master-code-dir nil)]
    +    (try
    +      (FileUtils/forceMkdir (File. tmproot))
    +      (.readBlobTo blob-store (master-stormcode-key storm-id) 
(FileOutputStream. (supervisor-stormcode-path tmproot)) nil)
    +      (.readBlobTo blob-store (master-stormconf-key storm-id) 
(FileOutputStream. (supervisor-stormconf-path tmproot)) nil)
    +      (finally
    +        (.shutdown blob-store)))
    +    (FileUtils/moveDirectory (File. tmproot) (File. stormroot))
    +    (setup-storm-code-dir conf (read-supervisor-storm-conf conf storm-id) 
stormroot)
    +    (let [classloader (.getContextClassLoader (Thread/currentThread))
    +          resources-jar (resources-jar)
    +          url (.getResource classloader RESOURCES-SUBDIR)
    +          target-dir (str stormroot file-path-separator RESOURCES-SUBDIR)]
    +      (cond
    +        resources-jar
    --- End diff --
    
    > param localizer is not used
    
    The `defmulti` `download-storm-code` has two methods, one for `:local` and 
one for `:distributed`. Both methods take the same number of parameters (4), 
but only use 3 of them. So in both methods there is an unused parameter.
    
    We could change to using 3 parameters, and just call the method with the 
correct third parameter (which we should know at the time we call it).
    
    ```Clojure
    (defmethod download-storm-code :local
      [conf storm-id master-code-dir]
    ```
    
    and 
    
    ```Clojure
    (defmethod download-storm-code :distributed
      [conf storm-id localizer]
    ```
    
    The dispatch function is `config/cluster-mode` and it only looks at the 
first argument (the conf).



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to