Pig
  1. Pig
  2. PIG-2660

PPNL should get notified of plan before it gets executed

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.11
    • Component/s: None
    • Labels:
      None
    • Release Note:
      Provide MROperPlan to PigProgressNotificationListener

      Description

      The PigProgressNotificationListner should get notified of the plan (MROperPlan) before it gets executed. This allows listeners to inspect the plan and have an idea what to expect in the execution flow. Proposal is to add the following method to the PPNL interface, which is marked as evolving:

      public void initialPlanNotification(MROperPlan plan);
      
      1. PIG-2660.2.patch
        7 kB
        Bill Graham
      2. PIG-2660.1.patch
        6 kB
        Bill Graham

        Activity

        Hide
        Bill Graham added a comment -

        Attaching a patch for review.

        Show
        Bill Graham added a comment - Attaching a patch for review.
        Hide
        Alan Gates added a comment -

        Looks good. Do we have any idea how often people implement PigProgressNotificationListerner? I'm wondering if the calls to initialPlanNotfication in SyncProgressNotificationAdaptor and ScriptState should be wrapped with a catch for NoSuchMethodException in case people are using old listeners. Other than that, +1.

        Show
        Alan Gates added a comment - Looks good. Do we have any idea how often people implement PigProgressNotificationListerner? I'm wondering if the calls to initialPlanNotfication in SyncProgressNotificationAdaptor and ScriptState should be wrapped with a catch for NoSuchMethodException in case people are using old listeners. Other than that, +1.
        Hide
        Bill Graham added a comment -

        Cool call. I haven't heard of anyone else using PPNLs on the list yet, but I agree with the suggestion. Patch #2 attached.

        I wondered if we should we take it a step further and catch Exception for all calls to a PPNL. Should a bug in a PPNL implementation cause a script to fail, possibly mid-way through execution?

        If we leave things as they are now, the answer would be yes, unless someone implemented their PPNL defensively to make it a no. As such, I'm inclined to leave things as they are and put the control in the hands of the implementer.

        Show
        Bill Graham added a comment - Cool call. I haven't heard of anyone else using PPNLs on the list yet, but I agree with the suggestion. Patch #2 attached. I wondered if we should we take it a step further and catch Exception for all calls to a PPNL. Should a bug in a PPNL implementation cause a script to fail, possibly mid-way through execution? If we leave things as they are now, the answer would be yes , unless someone implemented their PPNL defensively to make it a no . As such, I'm inclined to leave things as they are and put the control in the hands of the implementer.
        Hide
        Dmitriy V. Ryaboy added a comment -

        I tried to find users of the PPNL interface earlier, and none were to be had (Oozie is a potential one, but they said they don't use it). There are a number of changes to this interface we want to put into 0.11, they will be documented appropriately.

        Oozie asked that we implement an abstract PPNL for future-proofing, which I thought I did.. but now can't find. Outstanding jira?

        Show
        Dmitriy V. Ryaboy added a comment - I tried to find users of the PPNL interface earlier, and none were to be had (Oozie is a potential one, but they said they don't use it). There are a number of changes to this interface we want to put into 0.11, they will be documented appropriately. Oozie asked that we implement an abstract PPNL for future-proofing, which I thought I did.. but now can't find. Outstanding jira?
        Hide
        Bill Graham added a comment -

        Opened PIG-2678 for creating an abstract PPNL. Unless anyone else has comments, I'll commit patch #2.

        Show
        Bill Graham added a comment - Opened PIG-2678 for creating an abstract PPNL. Unless anyone else has comments, I'll commit patch #2.
        Hide
        Dmitriy V. Ryaboy added a comment -

        +1

        Show
        Dmitriy V. Ryaboy added a comment - +1
        Hide
        Bill Graham added a comment -

        Committed.

        Show
        Bill Graham added a comment - Committed.

          People

          • Assignee:
            Bill Graham
            Reporter:
            Bill Graham
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development