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

Provide a way to enable a post/* to run only under some context

      Problem:

      If you want to only send out notifications in a post/failure block for non pull-requests, you have currently to use the script block.

      Maybe a solution could be to allow when inside post/* (in the example below, having a way to express a default when would probably help make this more concise)

      Example:

      post {
          failure {
              script {
                  if (env.CHANGE_ID == null) {
                      mail to: NOTIFICATION_TARGET, subject: "FAILURE: ${currentBuild.fullDisplayName}", body: "Boo, we failed. $BUILD_URL"
                  }
              }
          }
          unstable {
              script {
                  if (env.CHANGE_ID == null) {
                      mail to: NOTIFICATION_TARGET, subject: "UNSTABLE: ${currentBuild.fullDisplayName}", body: "Huh, we're unstable. $BUILD_URL"
                  }
              }
          }
          changed {
              script {
                  if (env.CHANGE_ID == null && currentBuild.result == null) { // and JENKINS-41060
                      mail to: NOTIFICATION_TARGET, subject: "BACK TO SUCCESS: ${currentBuild.fullDisplayName}", body: "Yay! $BUILD_URL"
                  }
              }
          }
      }
      

      Proposals (WARNING: do not copy paste below if you just found that JIRA )

      Expressing the when each time

      post {
          failure {
              mail to: NOTIFICATION_TARGET, subject: "FAILURE: ${currentBuild.fullDisplayName}", body: "Boo, we failed. $BUILD_URL"
              when { expression { env.CHANGE_ID == null } }
          }
          unstable {
              mail to: NOTIFICATION_TARGET, subject: "UNSTABLE: ${currentBuild.fullDisplayName}", body: "Huh, we're unstable. $BUILD_URL"
              when { expression { env.CHANGE_ID == null } }
          }
          changed {
              mail to: NOTIFICATION_TARGET, subject: "BACK TO SUCCESS: ${currentBuild.fullDisplayName}", body: "Yay! $BUILD_URL"
              when { expression { env.CHANGE_ID == null && currentBuild.result == null } }
          }
      }
      

      Expressing the when as a default for every post/* blocks

      post {
          failure {
              mail to: NOTIFICATION_TARGET, subject: "FAILURE: ${currentBuild.fullDisplayName}", body: "Boo, we failed. $BUILD_URL"
          }
          unstable {
              mail to: NOTIFICATION_TARGET, subject: "UNSTABLE: ${currentBuild.fullDisplayName}", body: "Huh, we're unstable. $BUILD_URL"
          }
          changed {
              mail to: NOTIFICATION_TARGET, subject: "BACK TO SUCCESS: ${currentBuild.fullDisplayName}", body: "Yay! $BUILD_URL"
              when { expression { currentBuild.result == null } } // question arises then: should this expression be cumulative, or overriding the default below
          }
          when { expression { env.CHANGE_ID == null } }
      }
      

          [JENKINS-42688] Provide a way to enable a post/* to run only under some context

          Patrick Wolf added a comment -

          you don't need env you simply put CHANGE_ID == null

          Everything in post is already a conditional.  Is there a chance putting the when outside of the existing conditions could cause conflicts?

          Patrick Wolf added a comment - you don't need env you simply put CHANGE_ID == null Everything in post is already a conditional.  Is there a chance putting the when outside of the existing conditions could cause conflicts?

          Andrew Bayer added a comment -

          So the main issue here is that post conditions are simpler than when conditions, and unless I add nested post conditions in some form, like we have with when, they're always going to be less flexible than when conditions. I'm not saying that won't happen, but I'm not enthused about the idea.

          Andrew Bayer added a comment - So the main issue here is that post conditions are simpler than when conditions, and unless I add nested post conditions in some form, like we have with when , they're always going to be less flexible than when conditions. I'm not saying that won't happen, but I'm not enthused about the idea.

          Liam Newman added a comment - - edited

          I'm sorry for not giving feedback on this earlier. Mostly this feels to me like one more step toward Scripted pipeline.

          What is the win here? Once a user hits this level of complexity, it makes more sense for them to move the functionality into a shared library where they can use if statements. Strong -1 for this feature as whole, sorry.

          Assuming we've already decided this feature is going to happen, the name condition is too generic/overloaded. We already call the status-based directives "conditions" in the documentation.conditionalcomplexcomplex-condition? The fact that I can't think of a good term for this is further indication that this is not the place for this.

          abayer 

          Now that we have a JEP process, what do you think of making language changes like this into JEPs.  I know it would add some additional overhead, but it would also make people more aware of upcoming plans and start participating in the design earlier. 

           

          Liam Newman added a comment - - edited I'm sorry for not giving feedback on this earlier. Mostly this feels to me like one more step toward Scripted pipeline. What is the win here? Once a user hits this level of complexity, it makes more sense for them to move the functionality into a shared library where they can use if statements. Strong -1 for this feature as whole, sorry. Assuming we've already decided this feature is going to happen, the name  condition  is too generic/overloaded. We already call the status-based directives "conditions" in the documentation. conditional ?  complex ?  complex-condition ? The fact that I can't think of a good term for this is further indication that this is not the place for this. abayer   Now that we have a JEP process, what do you think of making language changes like this into JEPs.  I know it would add some additional overhead, but it would also make people more aware of upcoming plans and start participating in the design earlier.   

          James Nord added a comment -

          bitwiseman

          Once a user hits this level of complexity, it makes more sense for them to move the functionality into a shared library where they can use if statements. Strong -1 for this feature as whole, sorry.

          Complexity? you consider a mutibranch-pipeline complex?
          If you spam the entire team on every PR failure (PRs are expected to often fail) you will stop acting on genuine failures. This is pretty basic functionality IMO.

          Missing ways to fullfil the simple requirements like this is what forces people to either scripted or the script block which we should be trying to keep users away from.

          James Nord added a comment - bitwiseman Once a user hits this level of complexity, it makes more sense for them to move the functionality into a shared library where they can use if statements. Strong -1 for this feature as whole, sorry. Complexity? you consider a mutibranch-pipeline complex? If you spam the entire team on every PR failure (PRs are expected to often fail) you will stop acting on genuine failures. This is pretty basic functionality IMO. Missing ways to fullfil the simple requirements like this is what forces people to either scripted or the script block which we should be trying to keep users away from.

          Liam Newman added a comment -

          teiloN

          No part of my response said that I consider a multibranch pipeline complex.  I said complex conditional logic should go into a shared library step.

          The way to fulfill this simple requirement is not missing, it is present and provided by shared libraries. 

          I've already created blog post about around this case: https://jenkins.io/blog/2017/02/15/declarative-notifications/

          In my experience, the logic around sending notifications (or other behavior like what is show in this ticket) is always simple when it starts out. Then people very quickly realize they need a special case, and then another, and another.  Arguing that this simple case is sufficient reason to make Declarative more complex is misleading. Yes, this new feature handles this simple case, but then in short order users have to handle the next more complex one and run into the same problem you're trying to address.  Better to direct them to shared libraries sooner, giving them access to all the power they will need. 

          For example, while my blog post doesn't address this specific case, it is easy to add that functionality.  Whereas if I wanted to go from the scenario outlined in this bug to my case, I'd have to go to shared library or script anyway.

           

           

          Liam Newman added a comment - teilo N No part of my response said that I consider a multibranch pipeline complex.  I said complex conditional logic should go into a shared library step. The way to fulfill this simple requirement is not missing, it is present and provided by shared libraries.  I've already created blog post about around this case: https://jenkins.io/blog/2017/02/15/declarative-notifications/ In my experience, the logic around sending notifications (or other behavior like what is show in this ticket) is always simple when it starts out. Then people very quickly realize they need a special case, and then another, and another.  Arguing that this simple case is sufficient reason to make Declarative more complex is misleading. Yes, this new feature handles this simple case, but then in short order users have to handle the next more complex one and run into the same problem you're trying to address.  Better to direct them to shared libraries sooner, giving them access to all the power they will need.  For example, while my blog post doesn't address this specific case, it is easy to add that functionality.  Whereas if I wanted to go from the scenario outlined in this bug to my case, I'd have to go to shared library or script anyway.    

            Unassigned Unassigned
            batmat Baptiste Mathus
            Votes:
            2 Vote for this issue
            Watchers:
            8 Start watching this issue

              Created:
              Updated: