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

      When you copy a job it should be disabled by default so that any modifications
      can be made to it before it tries to run.

          [JENKINS-2494] Copied jobs should be disabled by default

          jpederzolli added a comment -

          I am currently working on the fix for this which follows the same approach as slaves, i.e. the cloned job will be in a temporary disabled state until the initial save. Unlike the initial patch in comment 1, the actual enabled/disabled status will be carried over from the job being cloned.

          Once this has been completed and tested internally (Yahoo!), I will look into merging upstream.

          jpederzolli added a comment - I am currently working on the fix for this which follows the same approach as slaves, i.e. the cloned job will be in a temporary disabled state until the initial save. Unlike the initial patch in comment 1, the actual enabled/disabled status will be carried over from the job being cloned. Once this has been completed and tested internally (Yahoo!), I will look into merging upstream.

          jpederzolli added a comment -

          Attaching patch which prevents a cloned build from running, via build trigger or manually, until the initial save.

          jpederzolli added a comment - Attaching patch which prevents a cloned build from running, via build trigger or manually, until the initial save.

          Alan Harder added a comment -

          looks pretty good to me.. 2 questions:
          1) should the variable be initialized to false somewhere?
          2) what if you copy a job and then leave the config screen w/o saving? is it ok to assume they'll come back and save it at some point to get it running? seems like this is ok, but thought i'd raise the question since there will be no indication why the job isn't running.. (I guess same question applies to copied slave nodes..)

          Alan Harder added a comment - looks pretty good to me.. 2 questions: 1) should the variable be initialized to false somewhere? 2) what if you copy a job and then leave the config screen w/o saving? is it ok to assume they'll come back and save it at some point to get it running? seems like this is ok, but thought i'd raise the question since there will be no indication why the job isn't running.. (I guess same question applies to copied slave nodes..)

          jpederzolli added a comment -

          1) should the variable be initialized to false somewhere?

          The boolean variable is going to implicitly initialized to false, so I don't think this is required (though wouldn't hurt).

          2) what if you copy a job and then leave the config screen w/o saving? is it ok to assume they'll come back and save it at some point to get it running? seems like this is ok, but thought i'd raise the question since there will be no indication why the job isn't running.. (I guess same question applies to copied slave nodes..)

          This is a fair point; my initial thoughts were that a) this is consistent with the copied slave behavior and b) I think most users would not want a duplicate job to run until changes are made.

          With these changes, a cloned build that is not saved will show up as gray in the UI indicating it is disabled. This will at least let the user know that a build is not going to take place, but may leave them wondering why.

          jpederzolli added a comment - 1) should the variable be initialized to false somewhere? The boolean variable is going to implicitly initialized to false, so I don't think this is required (though wouldn't hurt). 2) what if you copy a job and then leave the config screen w/o saving? is it ok to assume they'll come back and save it at some point to get it running? seems like this is ok, but thought i'd raise the question since there will be no indication why the job isn't running.. (I guess same question applies to copied slave nodes..) This is a fair point; my initial thoughts were that a) this is consistent with the copied slave behavior and b) I think most users would not want a duplicate job to run until changes are made. With these changes, a cloned build that is not saved will show up as gray in the UI indicating it is disabled. This will at least let the user know that a build is not going to take place, but may leave them wondering why.

          Code changed in hudson
          User: : jpederzolli
          Path:
          trunk/hudson/main/core/src/main/java/hudson/model/AbstractProject.java
          trunk/hudson/main/core/src/main/java/hudson/model/Job.java
          http://jenkins-ci.org/commit/31444
          Log:
          Issue: JENKINS-2494

          SCM/JIRA link daemon added a comment - Code changed in hudson User: : jpederzolli Path: trunk/hudson/main/core/src/main/java/hudson/model/AbstractProject.java trunk/hudson/main/core/src/main/java/hudson/model/Job.java http://jenkins-ci.org/commit/31444 Log: Issue: JENKINS-2494

          jpederzolli added a comment -

          committed patch after getting ok from Kohsuke.

          jpederzolli added a comment - committed patch after getting ok from Kohsuke.

          Code changed in hudson
          User: : mindless
          Path:
          trunk/www/changelog.html
          http://jenkins-ci.org/commit/31454
          Log:
          JENKINS-2494 note in change log

          SCM/JIRA link daemon added a comment - Code changed in hudson User: : mindless Path: trunk/www/changelog.html http://jenkins-ci.org/commit/31454 Log: JENKINS-2494 note in change log

          nealeu added a comment -

          I've just copied a Maven job and found that it ran with the existing before I'd saved it.

          Has the "Apply" button behaviour caused a regression here?

          nealeu added a comment - I've just copied a Maven job and found that it ran with the existing before I'd saved it. Has the "Apply" button behaviour caused a regression here?

          Code changed in jenkins
          User: Stephen Connolly
          Path:
          core/src/main/java/hudson/model/Job.java
          http://jenkins-ci.org/commit/jenkins/503c3bd2e6f2ec85514e16a260396ddae68f03ae
          Log:
          [FIXES JENKINS-2494] Restore correct behaviour

          • Fixes a regression in core where the display name clear on copy was triggering a save
          • More than one way to do this, could also have used the marker interface approach
            This route seems slightly less fragile, though people could still add ItemListeners
            with order == -Double.MAX_VALUE which would then introduce intdeterminism.
            A marker interface would remove that indeterminism as the onCopyComplete method would
            be only called on the Job as the last method... but it could be hard to
            ensure that all ItemGroupMixin's respect the calling of onCopyComplete contract
            hence this approach seems better to me for that reason

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Stephen Connolly Path: core/src/main/java/hudson/model/Job.java http://jenkins-ci.org/commit/jenkins/503c3bd2e6f2ec85514e16a260396ddae68f03ae Log: [FIXES JENKINS-2494] Restore correct behaviour Fixes a regression in core where the display name clear on copy was triggering a save More than one way to do this, could also have used the marker interface approach This route seems slightly less fragile, though people could still add ItemListeners with order == -Double.MAX_VALUE which would then introduce intdeterminism. A marker interface would remove that indeterminism as the onCopyComplete method would be only called on the Job as the last method... but it could be hard to ensure that all ItemGroupMixin's respect the calling of onCopyComplete contract hence this approach seems better to me for that reason

          dogfood added a comment -

          Integrated in jenkins_main_trunk #2687

          Result = UNSTABLE

          dogfood added a comment - Integrated in jenkins_main_trunk #2687 Result = UNSTABLE

            jpederzolli jpederzolli
            ajpurkiss ajpurkiss
            Votes:
            4 Vote for this issue
            Watchers:
            7 Start watching this issue

              Created:
              Updated:
              Resolved: