Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-291

a record needed why a build has started (what triggered it)

    • Icon: Improvement Improvement
    • Resolution: Fixed
    • Icon: Major Major
    • core
    • None
    • Platform: PC, OS: Linux

      It would be good to know what a build triggered. When a user triggered it via
      the web-site I would like to know who it was (that requires authentication).
      Other build reasons: a succeeded build of another project, a wget request by a
      script, a cron trigger.

        1. patch.291.feb8
          20 kB
        2. patch.291.feb5
          19 kB
        3. patch.issue291.4feb
          17 kB
        4. patch.291
          15 kB

          [JENKINS-291] a record needed why a build has started (what triggered it)

          abonko added a comment -

          Yes, being able to identify what triggered a build would be very useful.

          abonko added a comment - Yes, being able to identify what triggered a build would be very useful.

          bwestrich added a comment -

          +1
          I really miss in Hudson (CruiseControl has this feature, whew, how about flame
          bait!!!) the ability to see why a build happened. Was it because of an SCM poll,
          an upstream project build (if so, which one), a timed build, or a manual build
          (if so, by who)?

          bwestrich added a comment - +1 I really miss in Hudson (CruiseControl has this feature, whew, how about flame bait!!!) the ability to see why a build happened. Was it because of an SCM poll, an upstream project build (if so, which one), a timed build, or a manual build (if so, by who)?

              • Issue 1124 has been marked as a duplicate of this issue. ***

          Kohsuke Kawaguchi added a comment - Issue 1124 has been marked as a duplicate of this issue. ***

          adding myself as CC

          jameslivingston added a comment - adding myself as CC

          redsolo added a comment -

          The trigger record should contain a short reason and an optional longer command
          output (or description). The command output is interesting if a job is triggered
          by a SCM, and the user is wondering why the SCM triggered it. (See discussion
          http://www.nabble.com/Hudson-executes-build-even-though-no-SCM-changes-were-recorded-td18736261.html)

          redsolo added a comment - The trigger record should contain a short reason and an optional longer command output (or description). The command output is interesting if a job is triggered by a SCM, and the user is wondering why the SCM triggered it. (See discussion http://www.nabble.com/Hudson-executes-build-even-though-no-SCM-changes-were-recorded-td18736261.html )

          mdonohue added a comment -

          I'm taking a look at this. I think the best approach is to modify
          BuildableItem, and change the method 'boolean scheduleBuild()' to take an
          argument explaining why the build is being scheduled, then fix all callers to
          include an explanation, and add a UI element to show the reason.

          I have two questions though. One is how backward compatibility affects this. I
          think I would have to leave the old no-argument method in place, and have it
          give 'unknown' or 'legacy code' reason for launching the build. However, if
          it's possible to remove the old method, I would prefer to do that.

          The second question is what to store in the job itself. The boolean return
          value indicates whether the job is already in the queue. This means we could
          have several reasons for a job being built - the easy solution would be to
          record only the first reason, but the more complete solution would be to queue
          up all the reasons on the job, so they can all be browsed. I think I'll start
          with the easy route, and see about adding the list later.

          mdonohue added a comment - I'm taking a look at this. I think the best approach is to modify BuildableItem, and change the method 'boolean scheduleBuild()' to take an argument explaining why the build is being scheduled, then fix all callers to include an explanation, and add a UI element to show the reason. I have two questions though. One is how backward compatibility affects this. I think I would have to leave the old no-argument method in place, and have it give 'unknown' or 'legacy code' reason for launching the build. However, if it's possible to remove the old method, I would prefer to do that. The second question is what to store in the job itself. The boolean return value indicates whether the job is already in the queue. This means we could have several reasons for a job being built - the easy solution would be to record only the first reason, but the more complete solution would be to queue up all the reasons on the job, so they can all be browsed. I think I'll start with the easy route, and see about adding the list later.

          mdonohue added a comment -

          My second question is larger than I had realized. Browsing the code this
          evening, I'm seeing Task and Item objects as part of the queue manipulation
          code. On the model side, there are Job, Run, Project and Executor objects.
          This is more than I was considering, so I'd like to understand better the best
          place to store the reason for launching a job/project. I think the 'build
          reason' needs to end up on the Run object, but that object is not created until
          well after the project has been enqueued, so a series of storage places need to
          be identified.

          mdonohue added a comment - My second question is larger than I had realized. Browsing the code this evening, I'm seeing Task and Item objects as part of the queue manipulation code. On the model side, there are Job, Run, Project and Executor objects. This is more than I was considering, so I'd like to understand better the best place to store the reason for launching a job/project. I think the 'build reason' needs to end up on the Run object, but that object is not created until well after the project has been enqueued, so a series of storage places need to be identified.

          What you need is a class like ParameterizedProjectTask, that extends from
          QueueTaskFilter and works as a decorator to the newly created 'Executable'
          (which in this case is AbstractBuild.

          By implementing the hashCode and equals without taking the explanation into
          account, you can avoid stacking up builds in the queue (although that wouldn't
          allow you to do merging of the items — for that we need more work in the core.)

          Given that the parameterized build is in the core, too, I think
          ParameterizedProjectTask should be renamed and expanded to support everything we
          need to carry from the build scheduling to the actual build, including
          parameters, description, initiator, etc.

          Kohsuke Kawaguchi added a comment - What you need is a class like ParameterizedProjectTask, that extends from QueueTaskFilter and works as a decorator to the newly created 'Executable' (which in this case is AbstractBuild. By implementing the hashCode and equals without taking the explanation into account, you can avoid stacking up builds in the queue (although that wouldn't allow you to do merging of the items — for that we need more work in the core.) Given that the parameterized build is in the core, too, I think ParameterizedProjectTask should be renamed and expanded to support everything we need to carry from the build scheduling to the actual build, including parameters, description, initiator, etc.

          Alan Harder added a comment -

          I have working code for this, doing pretty much exactly what mdonahue described
          (w/o even reading this first, whoa). I'll review kohsuke's comment tomorrow and
          see how that might work.

          Alan Harder added a comment - I have working code for this, doing pretty much exactly what mdonahue described (w/o even reading this first, whoa). I'll review kohsuke's comment tomorrow and see how that might work.

          Alan Harder added a comment -

          started.

          Alan Harder added a comment - started.

          mdonohue added a comment -

          Nice. I did get Kohsuke's comment from Nov 17 implemented, but never finished
          the UI part.

          mdonohue added a comment - Nice. I did get Kohsuke's comment from Nov 17 implemented, but never finished the UI part.

          Alan Harder added a comment -

          I'm just writing "Build triggered by ..." in the job console output when the
          build starts, isn't that sufficient?

          Please attach a patch with your work, I'd be interested to see it... I looked a
          bit at ParameterizedProjectTask and it seemed like too many hoops to jump
          through to get the Task created everywhere a build can be scheduled.

          Alan Harder added a comment - I'm just writing "Build triggered by ..." in the job console output when the build starts, isn't that sufficient? Please attach a patch with your work, I'd be interested to see it... I looked a bit at ParameterizedProjectTask and it seemed like too many hoops to jump through to get the Task created everywhere a build can be scheduled.

          Alan Harder added a comment -

          Here's where I'm at:

          Outermost entry point: BuildableItem.. I don't see any alternative to adding a
          "triggeredBy" parameter here and deprecating the old methods. I've added
          convenience methods in Trigger (passing getDescriptor().getDisplayName() as
          triggeredBy) so this is pretty transparent for build triggers.

          Next level in: Queue/Queue.Task.. I started by also adding triggeredBy parameter
          in Queue.add(), but now I've refactored as kohsuke suggested, using
          ParameterizedProjectTask to hold the triggeredBy value (I didn't actually rename
          the class to something more generic yet).

          Listener: Added ItemListener.onScheduled(Queue.Task) and Queue.add() invokes
          this for each listener. Hudson registers one listener that remembers
          triggeredBy values for scheduled tasks, then writes the info into the console
          output when the build actually starts.

          Alan Harder added a comment - Here's where I'm at: Outermost entry point: BuildableItem.. I don't see any alternative to adding a "triggeredBy" parameter here and deprecating the old methods. I've added convenience methods in Trigger (passing getDescriptor().getDisplayName() as triggeredBy) so this is pretty transparent for build triggers. Next level in: Queue/Queue.Task.. I started by also adding triggeredBy parameter in Queue.add(), but now I've refactored as kohsuke suggested, using ParameterizedProjectTask to hold the triggeredBy value (I didn't actually rename the class to something more generic yet). Listener: Added ItemListener.onScheduled(Queue.Task) and Queue.add() invokes this for each listener. Hudson registers one listener that remembers triggeredBy values for scheduled tasks, then writes the info into the console output when the build actually starts.

          Code changed in hudson
          User: : mindless
          Path:
          trunk/hudson/main/core/src/main/java/hudson/matrix/MatrixBuild.java
          trunk/hudson/main/core/src/main/java/hudson/matrix/MatrixConfiguration.java
          trunk/hudson/main/core/src/main/java/hudson/maven/MavenBuild.java
          trunk/hudson/main/core/src/main/java/hudson/maven/MavenModuleSetBuild.java
          trunk/hudson/main/core/src/main/java/hudson/model/AbstractProject.java
          trunk/hudson/main/core/src/main/java/hudson/model/BuildableItem.java
          trunk/hudson/main/core/src/main/java/hudson/model/Hudson.java
          trunk/hudson/main/core/src/main/java/hudson/model/ParameterizedProjectTask.java
          trunk/hudson/main/core/src/main/java/hudson/model/ParametersDefinitionProperty.java
          trunk/hudson/main/core/src/main/java/hudson/model/Queue.java
          trunk/hudson/main/core/src/main/java/hudson/model/listeners/ItemListener.java
          trunk/hudson/main/core/src/main/java/hudson/tasks/BuildTrigger.java
          trunk/hudson/main/core/src/main/java/hudson/triggers/SCMTrigger.java
          trunk/hudson/main/core/src/main/java/hudson/triggers/TimerTrigger.java
          trunk/hudson/main/core/src/main/java/hudson/triggers/Trigger.java
          trunk/hudson/main/core/src/main/java/hudson/triggers/TriggerListener.java
          trunk/hudson/plugins/jabber/src/main/java/hudson/plugins/jabber/im/transport/bot/BuildCommand.java
          trunk/hudson/plugins/maven1-snapshot-plugin/src/main/java/hudson/plugins/mavensnapshottrigger/MavenSnapshotTrigger.java
          trunk/hudson/plugins/schedule-failed-builds/src/main/java/com/progress/hudson/ScheduleFailedBuildsTrigger.java
          trunk/hudson/plugins/url-change-trigger/src/main/java/com/redfin/hudson/UrlChangeTrigger.java
          trunk/www/changelog.html
          http://fisheye4.cenqua.com/changelog/hudson/?cs=14823
          Log:
          [FIXED JENKINS-291] Add new ItemListener.onScheduled(Queue.Task) interface method
          and invoke this from Queue.add(). Hudson registers a listener to write what
          triggered each build into the console output.
          BuildableItem.scheduleBuild() now takes triggeredBy parameter,
          ParameterizedProjectTask stores triggeredBy value for scheduled tasks,
          and Trigger has scheduleBuild convenience methods to use its descriptor's
          displayName as the triggeredBy value.

          SCM/JIRA link daemon added a comment - Code changed in hudson User: : mindless Path: trunk/hudson/main/core/src/main/java/hudson/matrix/MatrixBuild.java trunk/hudson/main/core/src/main/java/hudson/matrix/MatrixConfiguration.java trunk/hudson/main/core/src/main/java/hudson/maven/MavenBuild.java trunk/hudson/main/core/src/main/java/hudson/maven/MavenModuleSetBuild.java trunk/hudson/main/core/src/main/java/hudson/model/AbstractProject.java trunk/hudson/main/core/src/main/java/hudson/model/BuildableItem.java trunk/hudson/main/core/src/main/java/hudson/model/Hudson.java trunk/hudson/main/core/src/main/java/hudson/model/ParameterizedProjectTask.java trunk/hudson/main/core/src/main/java/hudson/model/ParametersDefinitionProperty.java trunk/hudson/main/core/src/main/java/hudson/model/Queue.java trunk/hudson/main/core/src/main/java/hudson/model/listeners/ItemListener.java trunk/hudson/main/core/src/main/java/hudson/tasks/BuildTrigger.java trunk/hudson/main/core/src/main/java/hudson/triggers/SCMTrigger.java trunk/hudson/main/core/src/main/java/hudson/triggers/TimerTrigger.java trunk/hudson/main/core/src/main/java/hudson/triggers/Trigger.java trunk/hudson/main/core/src/main/java/hudson/triggers/TriggerListener.java trunk/hudson/plugins/jabber/src/main/java/hudson/plugins/jabber/im/transport/bot/BuildCommand.java trunk/hudson/plugins/maven1-snapshot-plugin/src/main/java/hudson/plugins/mavensnapshottrigger/MavenSnapshotTrigger.java trunk/hudson/plugins/schedule-failed-builds/src/main/java/com/progress/hudson/ScheduleFailedBuildsTrigger.java trunk/hudson/plugins/url-change-trigger/src/main/java/com/redfin/hudson/UrlChangeTrigger.java trunk/www/changelog.html http://fisheye4.cenqua.com/changelog/hudson/?cs=14823 Log: [FIXED JENKINS-291] Add new ItemListener.onScheduled(Queue.Task) interface method and invoke this from Queue.add(). Hudson registers a listener to write what triggered each build into the console output. BuildableItem.scheduleBuild() now takes triggeredBy parameter, ParameterizedProjectTask stores triggeredBy value for scheduled tasks, and Trigger has scheduleBuild convenience methods to use its descriptor's displayName as the triggeredBy value.

          huybrechts added a comment -

          I can't really agree with how this is implemented...

          Mostly I think it is wrong to use a String to represent triggeredBy.
          We should have some structured information here (say a TriggerReason or BuildCause interface). This
          could contain as little as a string description, but build triggers that have more information can
          create their own implementations (e.g. containing a full User object instead of just "User foo").

          Furthermore this can be logged permanently as an Action in the Run so it is available in the remote
          API and can be used for display in the views for the Run.

          Logging the TriggerReason to the build output is still possible, but can be delegated to the
          TriggerReason itself.

          Since changing this after a release has implications for the persistence of queues, I hope this can
          be fixed before the next Hudson release, or this patch (temporarily) reverted.

          There are some further minor issues:

          • why is the triggeredBy for a run in a matrix build just "Matrix" and not the triggeredBy for the
            containing matrix run ?
          • the TriggerListener is a little weird. If the TriggerReason is attached to the build ,you can just
            add this log line in Run.run() where you have access to all information.

          A little more work would be to try to decompose the ParameterizedProjectTask again. Just a thought:
          how about if you could pass in
          a list of QueueTaskFilters (e.g. one with parameters and another with the TriggerReason) when you
          start a task. Then you can let the queue compose them and figure out if the build is already
          scheduled,
          and the QueueTaskFilters don't have to know about each other.

          huybrechts added a comment - I can't really agree with how this is implemented... Mostly I think it is wrong to use a String to represent triggeredBy. We should have some structured information here (say a TriggerReason or BuildCause interface). This could contain as little as a string description, but build triggers that have more information can create their own implementations (e.g. containing a full User object instead of just "User foo"). Furthermore this can be logged permanently as an Action in the Run so it is available in the remote API and can be used for display in the views for the Run. Logging the TriggerReason to the build output is still possible, but can be delegated to the TriggerReason itself. Since changing this after a release has implications for the persistence of queues, I hope this can be fixed before the next Hudson release, or this patch (temporarily) reverted. There are some further minor issues: why is the triggeredBy for a run in a matrix build just "Matrix" and not the triggeredBy for the containing matrix run ? the TriggerListener is a little weird. If the TriggerReason is attached to the build ,you can just add this log line in Run.run() where you have access to all information. A little more work would be to try to decompose the ParameterizedProjectTask again. Just a thought: how about if you could pass in a list of QueueTaskFilters (e.g. one with parameters and another with the TriggerReason) when you start a task. Then you can let the queue compose them and figure out if the build is already scheduled, and the QueueTaskFilters don't have to know about each other.

          I think I agree with Tom. A structure would be nice instead of just String.

          I also wonder if we should just extend Queue.Item to remember 'triggeredBy'
          information, rather than bypassing this via ParameterizedProjectTask. The way
          this change stands now, it makes it pointless for AbstractProject to extend from
          Queue.Task.

          My suggestion is to revert this change in the trunk, create a branch, apply the
          diff over there, and do some massaging.

          Kohsuke Kawaguchi added a comment - I think I agree with Tom. A structure would be nice instead of just String. I also wonder if we should just extend Queue.Item to remember 'triggeredBy' information, rather than bypassing this via ParameterizedProjectTask. The way this change stands now, it makes it pointless for AbstractProject to extend from Queue.Task. My suggestion is to revert this change in the trunk, create a branch, apply the diff over there, and do some massaging.

          Alan Harder added a comment -

          bummer, a simple string tells all I was interested in. matrix and maven just
          say "Matrix" and "Maven" mostly because I'm not familiar with those areas..
          weaving through all the interfaces, abstract classes and deprecated methods in
          hudson.model took long enough.. and now I'm afraid I'm out of time to work on
          this one, good luck..

          Alan Harder added a comment - bummer, a simple string tells all I was interested in. matrix and maven just say "Matrix" and "Maven" mostly because I'm not familiar with those areas.. weaving through all the interfaces, abstract classes and deprecated methods in hudson.model took long enough.. and now I'm afraid I'm out of time to work on this one, good luck..

          mdonohue added a comment -

          Oh, I stopped getting email updates once I was no longer the assignee, which is
          too bad. I've just resolved my work against the latest SVN, so should be able
          to attach a patch soon. I created a Cause object hierarchy, with subtypes
          LegacyCodeCause, UpstreamCause, TriggerCause, UserCause.

          These get dropped into a CauseAction that is attached to runs.

          All that went into hudson.model, which seems like a big package right now.

          mdonohue added a comment - Oh, I stopped getting email updates once I was no longer the assignee, which is too bad. I've just resolved my work against the latest SVN, so should be able to attach a patch soon. I created a Cause object hierarchy, with subtypes LegacyCodeCause, UpstreamCause, TriggerCause, UserCause. These get dropped into a CauseAction that is attached to runs. All that went into hudson.model, which seems like a big package right now.

          mdonohue added a comment -

          Created an attachment (id=537)
          Add a build cause, showing why each build was triggered

          mdonohue added a comment - Created an attachment (id=537) Add a build cause, showing why each build was triggered

          mdonohue added a comment -

          One issue I ran into is that using the parameterized objects means some of the
          UI elements show the project as parameterized. So that has to be tweaked as well.

          mdonohue added a comment - One issue I ran into is that using the parameterized objects means some of the UI elements show the project as parameterized. So that has to be tweaked as well.

          Alan Harder added a comment -

          super-quick review:

          • TriggerCause doesn't distinguish among different triggers
          • UserCause could have a no-param constructor, defaulting to
            Hudson.getAuthentication (or should UserCause store User object and default to
            User.current()?)
          • should base Cause class be abstract?

          Alan Harder added a comment - super-quick review: TriggerCause doesn't distinguish among different triggers UserCause could have a no-param constructor, defaulting to Hudson.getAuthentication (or should UserCause store User object and default to User.current()?) should base Cause class be abstract?

          mdonohue added a comment -

          Created an attachment (id=539)
          Updated patch to track why a build started

          mdonohue added a comment - Created an attachment (id=539) Updated patch to track why a build started

          mdonohue added a comment -

          The new patch adds a summary block to each job's view, so this change finally
          has a way of viewing the cause of a build. I'm using some generic orange square
          with a gear icon. I'd like something better, but have no candidates right now.

          I also took Alan's (mindless) suggestions into account. Cause is now abstract,
          and TriggerCause was replaced by SCMTriggerCause and TimerTriggerCause. I
          don't have any fields in those cause objects, since I don't know how to extract
          the appropriate information from Hudson. But it would be nice to include the
          polling log for the SCMTriggerCause object, and to include the Cron line that
          caused the trigger to start in the TimerTriggerCause object. Fortunately that
          can be added later with little pain.

          I no longer store an Authentication object, since that seemed to pull in a lot
          more than I was expecting. Instead, I'm just storing the username.

          I'd like to commit this in the next few days, so I'll solicit feedback on the
          developers list, and see where it goes.

          mdonohue added a comment - The new patch adds a summary block to each job's view, so this change finally has a way of viewing the cause of a build. I'm using some generic orange square with a gear icon. I'd like something better, but have no candidates right now. I also took Alan's (mindless) suggestions into account. Cause is now abstract, and TriggerCause was replaced by SCMTriggerCause and TimerTriggerCause. I don't have any fields in those cause objects, since I don't know how to extract the appropriate information from Hudson. But it would be nice to include the polling log for the SCMTriggerCause object, and to include the Cron line that caused the trigger to start in the TimerTriggerCause object. Fortunately that can be added later with little pain. I no longer store an Authentication object, since that seemed to pull in a lot more than I was expecting. Instead, I'm just storing the username. I'd like to commit this in the next few days, so I'll solicit feedback on the developers list, and see where it goes.

          jeffjensen added a comment -

          Add me to CC list.

          jeffjensen added a comment - Add me to CC list.

          Alan Harder added a comment -

          good progress, thanks. I have some more comments:

          • you removed scheduleBuild(int quietPeriod) from BuildableItem.. is this ok?
            or should it be deprecated, and a scheduleBuild(int quietPeriod, Cause cause) be
            added?
          • stackTrace in LegacyCodeCause is inaccessible.. I guess you put that there so
            one might find out where that legacy code is, but not sure what use it has if
            private and has no get method (unless maybe you go look at xml file where it is
            saved?)
          • regarding SCM and Timer cause classes.. so I guess the design will require
            every trigger plugin to implement a cause class? (ok I guess, just checking)
            Not sure if it would work, but would a TriggerCause containing a Trigger object
            be possible? Otherwise, Hudson uses a lot of inner classes, so maybe those 2
            could be inside the corresponding Trigger classes rather than more hudson.model
            classes. Regarding access to the cron schedule info, look at fields in the
            parent Trigger class.
          • javadoc in base Cause class?
          • javadoc on deprecated methods? (with @deprecated since {version}

            and what to
            use instead)

          • kohsuke's comment on my code still applies: "The way this change stands now,
            it makes it pointless for AbstractProject to extend from Queue.Task."
          • Still seems like UserCause could have a no param constructor for convenience..
            User object (User.current()) vs simple username?
          • The original ParameterizedProjectTask constructor is removed.. should it be
            kept (and deprecated) for compatibility?
          • Comment for ParameterizedProjectTask.hashCode:
            // cause is NOT included so different causes won't schedule duplicate builds
            Comment for ParameterizedProjectTask.equals:
            // cause is NOT checked so different causes won't schedule duplicate builds
          • TimerTrigger used to pass zero for quietPeriod, now it just passes a Cause..
            won't that be a behavior change? (now using the default quiet period)
          • should CauseAction and/or Cause use @ExportedBean,etc to add cause info in xml
            api?
          • updates for existing plugins?
          • there are 2 scheduleBuild calls in maven.MavenModuleSetBuild to be updated

          Alan Harder added a comment - good progress, thanks. I have some more comments: you removed scheduleBuild(int quietPeriod) from BuildableItem.. is this ok? or should it be deprecated, and a scheduleBuild(int quietPeriod, Cause cause) be added? stackTrace in LegacyCodeCause is inaccessible.. I guess you put that there so one might find out where that legacy code is, but not sure what use it has if private and has no get method (unless maybe you go look at xml file where it is saved?) regarding SCM and Timer cause classes.. so I guess the design will require every trigger plugin to implement a cause class? (ok I guess, just checking) Not sure if it would work, but would a TriggerCause containing a Trigger object be possible? Otherwise, Hudson uses a lot of inner classes, so maybe those 2 could be inside the corresponding Trigger classes rather than more hudson.model classes. Regarding access to the cron schedule info, look at fields in the parent Trigger class. javadoc in base Cause class? javadoc on deprecated methods? (with @deprecated since {version} and what to use instead) kohsuke's comment on my code still applies: "The way this change stands now, it makes it pointless for AbstractProject to extend from Queue.Task." Still seems like UserCause could have a no param constructor for convenience.. User object (User.current()) vs simple username? The original ParameterizedProjectTask constructor is removed.. should it be kept (and deprecated) for compatibility? Comment for ParameterizedProjectTask.hashCode: // cause is NOT included so different causes won't schedule duplicate builds Comment for ParameterizedProjectTask.equals: // cause is NOT checked so different causes won't schedule duplicate builds TimerTrigger used to pass zero for quietPeriod, now it just passes a Cause.. won't that be a behavior change? (now using the default quiet period) should CauseAction and/or Cause use @ExportedBean,etc to add cause info in xml api? updates for existing plugins? there are 2 scheduleBuild calls in maven.MavenModuleSetBuild to be updated

          mdonohue added a comment -
          • you removed scheduleBuild(int quietPeriod) from BuildableItem.. is this ok?
            or should it be deprecated, and a scheduleBuild(int quietPeriod, Cause cause) be
            added?

          I somehow misdirected the merge. I build the initial patch back in November of
          2008, and the quietPeriod method was added later.

          • stackTrace in LegacyCodeCause is inaccessible.. I guess you put that there so
            one might find out where that legacy code is, but not sure what use it has if
            private and has no get method (unless maybe you go look at xml file where it is
            saved?)

          Yeah, the stack trace is really only useful for development purposes. A plugin
          developer can take a look at the serialized XML to figure out where they're
          using the deprecated method, and provide their own cause.

          • regarding SCM and Timer cause classes.. so I guess the design will require
            every trigger plugin to implement a cause class? (ok I guess, just checking)

          It seems like it. We could put in a TextCause object, which just holds a string
          explanation, for use with simpler plugins. In general, it seems preferable to
          have an object model representation of the cause though.

          • Not sure if it would work, but would a TriggerCause containing a Trigger
            object be possible? Otherwise, Hudson uses a lot of inner classes, so maybe
            those 2 could be inside the corresponding Trigger classes rather than more
            hudson.model classes. Regarding access to the cron schedule info, look at
            fields in the parent Trigger class.

          It's certainly possible to put the trigger object into the TriggerCause, but it
          will probably lose it's relative link when serialized out to XML. If we go the
          inner class route, I'd prefer to put all the core Cause classes inside the Cause
          model object.

          • javadoc in base Cause class?

          OK

          • javadoc on deprecated methods? (with @deprecated since {version}

            and what to
            use instead)

          OK

          • kohsuke's comment on my code still applies: "The way this change stands now,
            it makes it pointless for AbstractProject to extend from Queue.Task."

          I'm going with Kohsuke's original suggestion to extend the
          ParameterizedProjectTask. Others have pointed out that having the object model
          right is more important than having a perfect code path, since the code can be
          updated later with low cost, compared to updating the object model. I'm going
          to stick with the current design on this.

          • Still seems like UserCause could have a no param constructor for convenience..
            User object (User.current()) vs simple username?

          OK

          • The original ParameterizedProjectTask constructor is removed.. should it be
            kept (and deprecated) for compatibility?

          Grepping the plugins folder finds one plugin that uses this constructor - jbpm,
          so I'll re-add it.

          • Comment for ParameterizedProjectTask.hashCode:
            // cause is NOT included so different causes won't schedule duplicate builds
            Comment for ParameterizedProjectTask.equals:
            // cause is NOT checked so different causes won't schedule duplicate builds

          OK

          • TimerTrigger used to pass zero for quietPeriod, now it just passes a Cause..
            won't that be a behavior change? (now using the default quiet period)

          Good catch. I must have resolved the conflict incorrectly here.

          • should CauseAction and/or Cause use @ExportedBean,etc to add cause info in xml
            api?

          I'm not familiar with the @ExportedBean annotation, and I haven't used the xml
          api. Should the annotation go on CauseAction or Cause?

          • updates for existing plugins?

          That's what the LegacyCodeCause is supposed to help facilitate. I haven't
          evaluated how many plugins make use of the now deprecated build code, but even
          once those changes are made, it seems a bit rude to release the plugins. I was
          planning to let the natural support, upgrade and build process take care of this.

          • there are 2 scheduleBuild calls in maven.MavenModuleSetBuild to be updated

          The downstream one is easy to fix. The other one I'm not sure, so I treated it
          as another downstream project from teh MavenModuleSetBuild.

          mdonohue added a comment - you removed scheduleBuild(int quietPeriod) from BuildableItem.. is this ok? or should it be deprecated, and a scheduleBuild(int quietPeriod, Cause cause) be added? I somehow misdirected the merge. I build the initial patch back in November of 2008, and the quietPeriod method was added later. stackTrace in LegacyCodeCause is inaccessible.. I guess you put that there so one might find out where that legacy code is, but not sure what use it has if private and has no get method (unless maybe you go look at xml file where it is saved?) Yeah, the stack trace is really only useful for development purposes. A plugin developer can take a look at the serialized XML to figure out where they're using the deprecated method, and provide their own cause. regarding SCM and Timer cause classes.. so I guess the design will require every trigger plugin to implement a cause class? (ok I guess, just checking) It seems like it. We could put in a TextCause object, which just holds a string explanation, for use with simpler plugins. In general, it seems preferable to have an object model representation of the cause though. Not sure if it would work, but would a TriggerCause containing a Trigger object be possible? Otherwise, Hudson uses a lot of inner classes, so maybe those 2 could be inside the corresponding Trigger classes rather than more hudson.model classes. Regarding access to the cron schedule info, look at fields in the parent Trigger class. It's certainly possible to put the trigger object into the TriggerCause, but it will probably lose it's relative link when serialized out to XML. If we go the inner class route, I'd prefer to put all the core Cause classes inside the Cause model object. javadoc in base Cause class? OK javadoc on deprecated methods? (with @deprecated since {version} and what to use instead) OK kohsuke's comment on my code still applies: "The way this change stands now, it makes it pointless for AbstractProject to extend from Queue.Task." I'm going with Kohsuke's original suggestion to extend the ParameterizedProjectTask. Others have pointed out that having the object model right is more important than having a perfect code path, since the code can be updated later with low cost, compared to updating the object model. I'm going to stick with the current design on this. Still seems like UserCause could have a no param constructor for convenience.. User object (User.current()) vs simple username? OK The original ParameterizedProjectTask constructor is removed.. should it be kept (and deprecated) for compatibility? Grepping the plugins folder finds one plugin that uses this constructor - jbpm, so I'll re-add it. Comment for ParameterizedProjectTask.hashCode: // cause is NOT included so different causes won't schedule duplicate builds Comment for ParameterizedProjectTask.equals: // cause is NOT checked so different causes won't schedule duplicate builds OK TimerTrigger used to pass zero for quietPeriod, now it just passes a Cause.. won't that be a behavior change? (now using the default quiet period) Good catch. I must have resolved the conflict incorrectly here. should CauseAction and/or Cause use @ExportedBean,etc to add cause info in xml api? I'm not familiar with the @ExportedBean annotation, and I haven't used the xml api. Should the annotation go on CauseAction or Cause? updates for existing plugins? That's what the LegacyCodeCause is supposed to help facilitate. I haven't evaluated how many plugins make use of the now deprecated build code, but even once those changes are made, it seems a bit rude to release the plugins. I was planning to let the natural support, upgrade and build process take care of this. there are 2 scheduleBuild calls in maven.MavenModuleSetBuild to be updated The downstream one is easy to fix. The other one I'm not sure, so I treated it as another downstream project from teh MavenModuleSetBuild.

          mdonohue added a comment -

          Created an attachment (id=542)
          Taking into account mindless's comments

          mdonohue added a comment - Created an attachment (id=542) Taking into account mindless's comments

          Alan Harder added a comment -

          wow, fast work.. i'll take a look at the latest tomorrow, but sounds like it
          should be getting quite close.

          • I didn't mean to suggest releasing all the affected plugins, but just checking
            in the changes so their next releases (whenever they happen) will have this
            change. But I'm fine with whatever the usual process for plugins is here.. (you
            can look at the file list for r14823 above for the plugins I found)
          • Still don't follow on the Queue.Task thing.. I too followed Kohsuke's
            suggestion, so it ends up with AbstractProject (which is a Queue.Task) never
            being passed directly to Queue.add(). Ok with me.. we'll see if anyone else
            comments on this part.
          • I'm not too sure on the @ExportedBean details either, I'd have to play with
            it.. try {job}

            /build/##/api/xml on a job. Since CauseAction is in the build, I
            think that should have @ExportedBean, then put @Exported on getCause()?
            something like that..

          Alan Harder added a comment - wow, fast work.. i'll take a look at the latest tomorrow, but sounds like it should be getting quite close. I didn't mean to suggest releasing all the affected plugins, but just checking in the changes so their next releases (whenever they happen) will have this change. But I'm fine with whatever the usual process for plugins is here.. (you can look at the file list for r14823 above for the plugins I found) Still don't follow on the Queue.Task thing.. I too followed Kohsuke's suggestion, so it ends up with AbstractProject (which is a Queue.Task) never being passed directly to Queue.add(). Ok with me.. we'll see if anyone else comments on this part. I'm not too sure on the @ExportedBean details either, I'd have to play with it.. try {job} /build/##/api/xml on a job. Since CauseAction is in the build, I think that should have @ExportedBean, then put @Exported on getCause()? something like that..

          Alan Harder added a comment -

          looks great..
          could save a couple lines in ParameterizedProjectTask's deprecated constructor
          by calling this(project, parameters, new Cause.LegacyCodeCause());

          Alan Harder added a comment - looks great.. could save a couple lines in ParameterizedProjectTask's deprecated constructor by calling this(project, parameters, new Cause.LegacyCodeCause());

          Alan Harder added a comment -

          something to test:
          set the quiet period high enough that you'll see jobs waiting in the queue
          before the build actually starts... what happens when you click on an item here?
          I think previously it would go to that job (project itself is in the queue),
          but when wrapped in ParameterizedProjectTask, does the link work?

          Alan Harder added a comment - something to test: set the quiet period high enough that you'll see jobs waiting in the queue before the build actually starts... what happens when you click on an item here? I think previously it would go to that job (project itself is in the queue), but when wrapped in ParameterizedProjectTask, does the link work?

          huybrechts added a comment -

          Looks good...

          One detail: UpstreamCause.upstreamCause is stored but never used. If you need it
          you can just ask the upstream build.

          These could be done later:

          • add "AbstractBuild UpstreamCause.getBuild()" and "User UserCause.getUser()".
            This will get mapped by stapler so you could do /job/XYZ/1/cause/build. I'm sure
            somebody will have a use for this.
          • @ExportedBean goes onto classes you want in the xml api (i.e. your concrete
            cause classes), and @Exported onto their getters (which should be added too)

          huybrechts added a comment - Looks good... One detail: UpstreamCause.upstreamCause is stored but never used. If you need it you can just ask the upstream build. These could be done later: add "AbstractBuild UpstreamCause.getBuild()" and "User UserCause.getUser()". This will get mapped by stapler so you could do /job/XYZ/1/cause/build. I'm sure somebody will have a use for this. @ExportedBean goes onto classes you want in the xml api (i.e. your concrete cause classes), and @Exported onto their getters (which should be added too)

          mdonohue added a comment -

          I simplified the deprecated constructor as suggested for ParameterizedProjectTask

          Regarding the quiet period, I induced a change while setting the quiet period to
          120 seconds. I don't see an accessible build page via the UI, so I don't think
          this is an issue - the job isn't accessible until the quiet period expired.

          mdonohue added a comment - I simplified the deprecated constructor as suggested for ParameterizedProjectTask Regarding the quiet period, I induced a change while setting the quiet period to 120 seconds. I don't see an accessible build page via the UI, so I don't think this is an issue - the job isn't accessible until the quiet period expired.

          mdonohue added a comment -

          Regarding the upstream cause, I wanted to store the chain of reasons for a given
          build in the build itself. That way conflicting expiration policies won't
          prevent the user from understanding the derivation of an existing job.

          For example, if the upstream job expires builds quickly, you will lose the
          causal chain on the downstream jobs.

          This information is not presently exposed via the UI, but I think the current UI
          for this feature is pretty primitive, and will get more rich as others use it
          and think of better mechanisms for presentation.

          I'll add the XML annotations and then commit this patch.

          mdonohue added a comment - Regarding the upstream cause, I wanted to store the chain of reasons for a given build in the build itself. That way conflicting expiration policies won't prevent the user from understanding the derivation of an existing job. For example, if the upstream job expires builds quickly, you will lose the causal chain on the downstream jobs. This information is not presently exposed via the UI, but I think the current UI for this feature is pretty primitive, and will get more rich as others use it and think of better mechanisms for presentation. I'll add the XML annotations and then commit this patch.

          Code changed in hudson
          User: : mdonohue
          Path:
          trunk/hudson/main/core/src/main/java/hudson/matrix/MatrixBuild.java
          trunk/hudson/main/core/src/main/java/hudson/matrix/MatrixConfiguration.java
          trunk/hudson/main/core/src/main/java/hudson/maven/MavenBuild.java
          trunk/hudson/main/core/src/main/java/hudson/maven/MavenModuleSetBuild.java
          trunk/hudson/main/core/src/main/java/hudson/model/AbstractProject.java
          trunk/hudson/main/core/src/main/java/hudson/model/BuildableItem.java
          trunk/hudson/main/core/src/main/java/hudson/model/Cause.java
          trunk/hudson/main/core/src/main/java/hudson/model/CauseAction.java
          trunk/hudson/main/core/src/main/java/hudson/model/ParameterizedProjectTask.java
          trunk/hudson/main/core/src/main/java/hudson/model/ParametersDefinitionProperty.java
          trunk/hudson/main/core/src/main/java/hudson/tasks/BuildTrigger.java
          trunk/hudson/main/core/src/main/java/hudson/triggers/SCMTrigger.java
          trunk/hudson/main/core/src/main/java/hudson/triggers/TimerTrigger.java
          trunk/hudson/main/core/src/main/resources/hudson/model/CauseAction/summary.jelly
          http://fisheye4.cenqua.com/changelog/hudson/?cs=15150
          Log:
          Fix JENKINS-291
          We now add a CauseAction to each job, and a summary.jelly file which renders
          the cause on any job with an explanation.

          SCM/JIRA link daemon added a comment - Code changed in hudson User: : mdonohue Path: trunk/hudson/main/core/src/main/java/hudson/matrix/MatrixBuild.java trunk/hudson/main/core/src/main/java/hudson/matrix/MatrixConfiguration.java trunk/hudson/main/core/src/main/java/hudson/maven/MavenBuild.java trunk/hudson/main/core/src/main/java/hudson/maven/MavenModuleSetBuild.java trunk/hudson/main/core/src/main/java/hudson/model/AbstractProject.java trunk/hudson/main/core/src/main/java/hudson/model/BuildableItem.java trunk/hudson/main/core/src/main/java/hudson/model/Cause.java trunk/hudson/main/core/src/main/java/hudson/model/CauseAction.java trunk/hudson/main/core/src/main/java/hudson/model/ParameterizedProjectTask.java trunk/hudson/main/core/src/main/java/hudson/model/ParametersDefinitionProperty.java trunk/hudson/main/core/src/main/java/hudson/tasks/BuildTrigger.java trunk/hudson/main/core/src/main/java/hudson/triggers/SCMTrigger.java trunk/hudson/main/core/src/main/java/hudson/triggers/TimerTrigger.java trunk/hudson/main/core/src/main/resources/hudson/model/CauseAction/summary.jelly http://fisheye4.cenqua.com/changelog/hudson/?cs=15150 Log: Fix JENKINS-291 We now add a CauseAction to each job, and a summary.jelly file which renders the cause on any job with an explanation.

          mdonohue added a comment -

          Created an attachment (id=546)
          Final patch - this is now committed as revision 15150

          mdonohue added a comment - Created an attachment (id=546) Final patch - this is now committed as revision 15150

          mdonohue added a comment -

          Now that we're committed, I'm marking this fixed.

          mdonohue added a comment - Now that we're committed, I'm marking this fixed.

          Alan Harder added a comment -

          this should be mentioned in changelog.html, and check the unit tests for new
          breakages..

          Alan Harder added a comment - this should be mentioned in changelog.html, and check the unit tests for new breakages..

          Code changed in hudson
          User: : mindless
          Path:
          trunk/hudson/main/core/src/main/java/hudson/model/Cause.java
          trunk/hudson/main/core/src/main/java/hudson/model/CauseAction.java
          trunk/hudson/main/core/src/main/java/hudson/triggers/SCMTrigger.java
          trunk/hudson/main/core/src/main/java/hudson/triggers/TimerTrigger.java
          http://fisheye4.cenqua.com/changelog/hudson/?cs=15237
          Log:
          JENKINS-291 Export CauseAction so exported Causes can be accessed via /api/.
          Export base Cause class and its short description instead of on every subclass.

          SCM/JIRA link daemon added a comment - Code changed in hudson User: : mindless Path: trunk/hudson/main/core/src/main/java/hudson/model/Cause.java trunk/hudson/main/core/src/main/java/hudson/model/CauseAction.java trunk/hudson/main/core/src/main/java/hudson/triggers/SCMTrigger.java trunk/hudson/main/core/src/main/java/hudson/triggers/TimerTrigger.java http://fisheye4.cenqua.com/changelog/hudson/?cs=15237 Log: JENKINS-291 Export CauseAction so exported Causes can be accessed via /api/. Export base Cause class and its short description instead of on every subclass.

          ejono added a comment -

          This is awesome! Thank you very much for doing this. However, it would be much
          nicer if it displayed upstream builds as a link to that particular build rather
          than just a textual description.

          ejono added a comment - This is awesome! Thank you very much for doing this. However, it would be much nicer if it displayed upstream builds as a link to that particular build rather than just a textual description.

          mdonohue added a comment -

          > it would be much nicer if it displayed upstream builds as a link
          > to that particular build rather than just a textual description.

          This would best be handled by a new enhancement request, or becoming a
          contributor yourself.

          mdonohue added a comment - > it would be much nicer if it displayed upstream builds as a link > to that particular build rather than just a textual description. This would best be handled by a new enhancement request, or becoming a contributor yourself.

          ejono added a comment -

          Yeah, I guess so. Maybe once I graduate I'll become a contributor.

          ejono added a comment - Yeah, I guess so. Maybe once I graduate I'll become a contributor.

          Alan Harder added a comment -

          that request is issue #3054

          Alan Harder added a comment - that request is issue #3054

          mdonohue added a comment -
              • Issue 2123 has been marked as a duplicate of this issue. ***

          mdonohue added a comment - Issue 2123 has been marked as a duplicate of this issue. ***

          mdonohue added a comment -
              • Issue 2522 has been marked as a duplicate of this issue. ***

          mdonohue added a comment - Issue 2522 has been marked as a duplicate of this issue. ***

            mdonohue mdonohue
            gradopado gradopado
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated:
              Resolved: