[ 
https://issues.apache.org/jira/browse/STORM-2016?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15413289#comment-15413289
 ] 

ASF GitHub Bot commented on STORM-2016:
---------------------------------------

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

    https://github.com/apache/storm/pull/1608#discussion_r74027328
  
    --- Diff: storm-core/src/jvm/org/apache/storm/StormSubmitter.java ---
    @@ -238,39 +239,24 @@ public static void submitTopologyAs(String name, Map 
stormConf, StormTopology to
                     }
     
                     // Dependency uploading only makes sense for distributed 
mode
    -                List<String> jarsBlobKeys;
    +                List<String> jarsBlobKeys = Collections.emptyList();
                     List<String> artifactsBlobKeys;
     
                     DependencyUploader uploader = new DependencyUploader();
                     try {
                         uploader.init();
                         jarsBlobKeys = 
uploadDependencyJarsToBlobStore(uploader);
                         artifactsBlobKeys = 
uploadDependencyArtifactsToBlobStore(uploader);
    +                    setDependencyBlobsToTopology(topology, jarsBlobKeys, 
artifactsBlobKeys);
    +                    submitTopologyInDistributeMode(name, topology, opts, 
progressListener, asUser, conf, serConf);
    +                } catch (Throwable e) {
    +                    // remove uploaded jars blobs, not artifacts since 
they're shared across the cluster
    +                    uploader.deleteBlobs(jarsBlobKeys);
    --- End diff --
    
    @satishd 
    In fact, submitTopologyWithOpts() can also throw AuthorizationException and 
org.apache.thrift.TException, too.
    
    Let's back to flow of logic.
    Please note that we didn't catch Throwable (some known Exceptions and also 
RuntimeException) and just pass to caller. And there's also no return for 
Nimbus.Client.submitTopology(), which indicates that unless we get TException 
or other Exceptions it should be treated as succeed. (If not we can't guarantee 
topology deploy is succeed.)
    
    I think this assumption is right for now, and assumption should be right 
unless we add some operations after calling submitTopologyInDistributeMode() or 
adding some operations after logging "Finished submitting topology".


> Topology submission improvement: support adding local jars and maven 
> artifacts on submission
> --------------------------------------------------------------------------------------------
>
>                 Key: STORM-2016
>                 URL: https://issues.apache.org/jira/browse/STORM-2016
>             Project: Apache Storm
>          Issue Type: Improvement
>          Components: storm-core
>            Reporter: Jungtaek Lim
>            Assignee: Jungtaek Lim
>
> This JIRA tracks actual work on below proposal / design document.
> https://cwiki.apache.org/confluence/display/STORM/A.+Design+doc%3A+adding+jars+and+maven+artifacts+at+submission
> Proposal discussion thread is here: 
> http://mail-archives.apache.org/mod_mbox/storm-dev/201608.mbox/%3ccaf5108i9+tjanz0lgrktmkvqel7f+53k9uyzxct6zhsu6oh...@mail.gmail.com%3E
> Let's post on discussion thread if we have any opinions / ideas on this 
> instead of leaving comments on this issue.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to