You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by Ethanlm <gi...@git.apache.org> on 2017/10/03 19:00:37 UTC

[GitHub] storm pull request #2355: [STORM-2769] Fast-fail if output streamId is null

GitHub user Ethanlm opened a pull request:

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

    [STORM-2769] Fast-fail if output streamId is null

    JIRA: https://issues.apache.org/jira/browse/STORM-2769
    
    If streamId is null, we should throw an exception because thrift doesn't support null in map keys.
    
    A test:
    
    ![image](https://user-images.githubusercontent.com/14900612/31143518-188d8a32-a843-11e7-800d-045219948008.png)
    


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

    $ git pull https://github.com/Ethanlm/storm STORM-2769

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

    https://github.com/apache/storm/pull/2355.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 #2355
    
----
commit ec85f643c5ace9bc1f454f2c33689b97af357e4a
Author: Ethan Li <et...@gmail.com>
Date:   2017-10-03T18:54:18Z

    [STORM-2769] Fast-fail if output streamId is null

----


---

[GitHub] storm issue #2355: [STORM-2769] Fast-fail if output streamId is null

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

    https://github.com/apache/storm/pull/2355
  
    Seems reasonable to me. +1


---

[GitHub] storm pull request #2355: [STORM-2769] Fast-fail if output streamId is null

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

    https://github.com/apache/storm/pull/2355#discussion_r142506999
  
    --- Diff: storm-client/src/jvm/org/apache/storm/topology/OutputFieldsGetter.java ---
    @@ -39,6 +39,9 @@ public void declareStream(String streamId, Fields fields) {
         }
     
         public void declareStream(String streamId, boolean direct, Fields fields) {
    +        if (null == streamId) {
    +            throw new IllegalArgumentException("streamId cann't be null");
    --- End diff --
    
    Thanks. I made a really silly mistake...


---

[GitHub] storm pull request #2355: [STORM-2769] Fast-fail if output streamId is null

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

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


---

[GitHub] storm pull request #2355: [STORM-2769] Fast-fail if output streamId is null

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

    https://github.com/apache/storm/pull/2355#discussion_r142506512
  
    --- Diff: storm-client/src/jvm/org/apache/storm/topology/OutputFieldsGetter.java ---
    @@ -39,6 +39,9 @@ public void declareStream(String streamId, Fields fields) {
         }
     
         public void declareStream(String streamId, boolean direct, Fields fields) {
    +        if (null == streamId) {
    +            throw new IllegalArgumentException("streamId cann't be null");
    --- End diff --
    
    Nit: cann't with two n's


---

[GitHub] storm issue #2355: [STORM-2769] Fast-fail if output streamId is null

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

    https://github.com/apache/storm/pull/2355
  
    Thanks @Ethanlm, merged to master, 1.x and 1.1.x.


---