Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.7.0
    • Component/s: core
    • Labels:
      None

      Description

      ScriptBasedClusterAction is using NPE for flow control.

      public Cluster execute(ClusterSpec clusterSpec, Cluster cluster) throws IOException, InterruptedException {
      ...
      for (String role : instanceTemplate.getRoles()) {
      try

      { handlerMap.get(role).beforeAction(event); }

      catch (NullPointerException e)

      { throw new IllegalArgumentException("No handler for role " + role); }

      }
      ...
      }

      I just lost about an hour trying to find out why the actionhandler was not being registered while the NPE came from the beforeAction() call.

      1. WHIRR-394.patch
        8 kB
        Andrei Savu
      2. WHIRR-394.patch
        6 kB
        Andrei Savu
      3. WHIRR-394.patch
        0.9 kB
        David Alves

        Activity

        David Alves created issue -
        Hide
        David Alves added a comment -

        Simple fix that tests if there is a handler first (not catching an NPE)

        Show
        David Alves added a comment - Simple fix that tests if there is a handler first (not catching an NPE)
        David Alves made changes -
        Field Original Value New Value
        Status Open [ 1 ] Patch Available [ 10002 ]
        David Alves made changes -
        Attachment WHIRR-394.patch [ 12498177 ]
        Hide
        Andrei Savu added a comment -

        +1 - too bad we've missed this in previous reviews.

        Show
        Andrei Savu added a comment - +1 - too bad we've missed this in previous reviews.
        Hide
        Andrei Savu added a comment -

        It seems like the ComputingMap returned by HandleMapFactory.create does not work with null values. I have updated the code to throw an exception with a relevant error message. All unit tests are passing with this change.

        http://code.google.com/p/google-collections/issues/detail?id=166

        Show
        Andrei Savu added a comment - It seems like the ComputingMap returned by HandleMapFactory.create does not work with null values. I have updated the code to throw an exception with a relevant error message. All unit tests are passing with this change. http://code.google.com/p/google-collections/issues/detail?id=166
        Andrei Savu made changes -
        Attachment WHIRR-394.patch [ 12498430 ]
        Hide
        David Alves added a comment -

        There is another instance of the bug a bit further down in ScriptBasedCluster action.
        Andrei could you update that too?
        (more of a code quality thing as there is no way an NPE would be thrown there).

        Show
        David Alves added a comment - There is another instance of the bug a bit further down in ScriptBasedCluster action. Andrei could you update that too? (more of a code quality thing as there is no way an NPE would be thrown there).
        Hide
        Andrei Savu added a comment -

        In this update I have extracted a method for handling this error condition. All unit tests are passing.

        Show
        Andrei Savu added a comment - In this update I have extracted a method for handling this error condition. All unit tests are passing.
        Andrei Savu made changes -
        Attachment WHIRR-394.patch [ 12498432 ]
        Hide
        Andrei Savu added a comment -

        I just did a search and I think this is the only place were we are catching NullPointerException.

        Show
        Andrei Savu added a comment - I just did a search and I think this is the only place were we are catching NullPointerException.
        Andrei Savu made changes -
        Status Patch Available [ 10002 ] In Progress [ 3 ]
        Assignee David Alves [ dr-alves ]
        Fix Version/s 0.7.0 [ 12317571 ]
        Hide
        Andrei Savu added a comment -

        Good to go?

        Show
        Andrei Savu added a comment - Good to go?
        Hide
        David Alves added a comment -

        +1 applies cleanly unit test pass.

        Show
        David Alves added a comment - +1 applies cleanly unit test pass.
        Hide
        Andrei Savu added a comment -

        Committed. Thanks David for reviewing.

        Show
        Andrei Savu added a comment - Committed. Thanks David for reviewing.
        Andrei Savu made changes -
        Status In Progress [ 3 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Patch Available Patch Available
        4m 25s 1 David Alves 07/Oct/11 15:47
        Patch Available Patch Available In Progress In Progress
        3d 19h 54m 1 Andrei Savu 11/Oct/11 11:41
        In Progress In Progress Resolved Resolved
        3d 3h 48m 1 Andrei Savu 14/Oct/11 15:30

          People

          • Assignee:
            David Alves
            Reporter:
            David Alves
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development