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

Large number of build parameters for pending jobs (e.g.gerrit triggered job) can cause unwieldy build history

    XMLWordPrintable

Details

    Description

      If Gerrit triggers create a queue in the Pending Jobs column in the project page, it will insert build parameters/metadata about the trigger: GERRIT_EVENT_TYPE, GERRIT_EVENT_HASH, etc.

      If this metadata is long, it will distort the project page to be visually unusable.

      Would like a way to either turn of or truncate this display of Gerrit metadata.

      Attachments

        Issue Links

          Activity

            rin_ne rin_ne added a comment -

            "metadata is long" means that a parameter has long value?
            If so, I think this is mainly jenkins issue.

            AFAIK, it is GERRIT_CHANGE_COMMIT_MESSAGE only. Because this is multi-line text, others are single-line.
            So StringParameterValue class should be replaced to TextParameterValue.

            But it would not be solved. ParameterValue which is extended by both StringParameterValue and
            TextParameterValue has 2 methods for description.
            In Javadoc, getShortDescription() requires one-line string as return value. I think this is truncation spec
            you mentioned.
            http://javadoc.jenkins-ci.org/hudson/model/ParameterValue.html#getShortDescription()

            But, as of now, TextParameterValue which extends StringParameterValue does not have any overrided
            methods for getShortDescription().
            https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/model/TextParameterValue.java

            It means that TextParameterValue#getShortDescription returns the same value as StringParameterValue's one.

            So, unfortunately, it is hard to solve this issue only by plugin fixes.

            If "metadata is long" means a lot of parameters, it is also Jenkins issue.
            Jenkins does not have the way to reduce the display number of parameters from plugin side.

            rin_ne rin_ne added a comment - "metadata is long" means that a parameter has long value? If so, I think this is mainly jenkins issue. AFAIK, it is GERRIT_CHANGE_COMMIT_MESSAGE only. Because this is multi-line text, others are single-line. So StringParameterValue class should be replaced to TextParameterValue. But it would not be solved. ParameterValue which is extended by both StringParameterValue and TextParameterValue has 2 methods for description. In Javadoc, getShortDescription() requires one-line string as return value. I think this is truncation spec you mentioned. http://javadoc.jenkins-ci.org/hudson/model/ParameterValue.html#getShortDescription( ) But, as of now, TextParameterValue which extends StringParameterValue does not have any overrided methods for getShortDescription(). https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/model/TextParameterValue.java It means that TextParameterValue#getShortDescription returns the same value as StringParameterValue's one. So, unfortunately, it is hard to solve this issue only by plugin fixes. If "metadata is long" means a lot of parameters, it is also Jenkins issue. Jenkins does not have the way to reduce the display number of parameters from plugin side.
            eric_griswold Eric Griswold added a comment -

            Here's a screenshot.

            eric_griswold Eric Griswold added a comment - Here's a screenshot.
            rin_ne rin_ne added a comment -

            From screenshot, this issue is not "long string in a parameter" but "too many parameters". Is it correct?

            If so, plugin does not have any solution to fix this.

            rin_ne rin_ne added a comment - From screenshot, this issue is not "long string in a parameter" but "too many parameters". Is it correct? If so, plugin does not have any solution to fix this.
            eric_griswold Eric Griswold added a comment -

            Looks like either or both. I keep trying to find a way to send this to core development. If it gets redirected back to you, please let me know what I'm doing wrong.

            eric_griswold Eric Griswold added a comment - Looks like either or both. I keep trying to find a way to send this to core development. If it gets redirected back to you, please let me know what I'm doing wrong.

            any updates on the issue?

            It makes the queue overview almost impossible to manage.

            bookwar Aleksandra Fedorova added a comment - any updates on the issue? It makes the queue overview almost impossible to manage.

            This issue still exist in

            • Jenkins ver. 1.563
            • Gerrit Trigger ver. 2.11.1
            jakub Jakub Czaplicki added a comment - This issue still exist in Jenkins ver. 1.563 Gerrit Trigger ver. 2.11.1
            jakub Jakub Czaplicki added a comment - - edited

            After looking at the plugin code and googling a bit it seems that it's not the plugin to decide on the length of paramters shown for pending jobs.
            It seems that it's the jenkins core controls this. Am I right ?

            I've added new jira for this issue: JENKINS-23064

            jakub Jakub Czaplicki added a comment - - edited After looking at the plugin code and googling a bit it seems that it's not the plugin to decide on the length of paramters shown for pending jobs. It seems that it's the jenkins core controls this. Am I right ? I've added new jira for this issue: JENKINS-23064
            rin_ne rin_ne added a comment -

            Yes. In plugin side, There is no way to truncate the display length of entire parameters. Also plugin cannot reduce the number of parameters. Because big impact to existing jobs.

            As a side note, a patch that changes the parameter type of GERRIT_CHANGE_COMMIT_MESSAGE to TextParameterValue is already merged.

            rin_ne rin_ne added a comment - Yes. In plugin side, There is no way to truncate the display length of entire parameters. Also plugin cannot reduce the number of parameters. Because big impact to existing jobs. As a side note, a patch that changes the parameter type of GERRIT_CHANGE_COMMIT_MESSAGE to TextParameterValue is already merged.
            jakub Jakub Czaplicki added a comment - - edited

            The file and the line that require modification :
            https://github.com/jenkinsci/jenkins/blob/master/core/src/main/resources/hudson/widgets/BuildHistoryWidget/entries.jelly#L59

            I suggest to use the jQuery Expander Plugin or a similar solution if jQuery is not built in in Jenkins core:
            http://plugins.learningjquery.com/expander/demo/index.html
            http://plugins.learningjquery.com/expander/index.html

            jakub Jakub Czaplicki added a comment - - edited The file and the line that require modification : https://github.com/jenkinsci/jenkins/blob/master/core/src/main/resources/hudson/widgets/BuildHistoryWidget/entries.jelly#L59 I suggest to use the jQuery Expander Plugin or a similar solution if jQuery is not built in in Jenkins core: http://plugins.learningjquery.com/expander/demo/index.html http://plugins.learningjquery.com/expander/index.html
            jakub Jakub Czaplicki added a comment - - edited

            I did some work on this. The idea is to hide the build parameters and only show them when a user clicks on the "pending—Build #..." string, with ability to hide the long string again (and later with ability to view/expand all hidden build parameters). The working example script of what I aim for is here: http://jsfiddle.net/jakubczaplicki/WT88W/

            It required modification of two files (see diffs below).

            I'd like a feedback and some suggestions/help on how to improve this code, such that it can be applied to master branch.

            Some notes:

            • Tested on Jenkins ver. 1.565
            • Uses jQuery 1.7.2 (which is AFAIK default jQuery version built in jenkins)
            • Every N seconds the BuildHistory is being refreshed. After each refresh, the parameters are hidden again.
            • I am aware that I used deprecated jQuery's .live() method, however the suggestion on http://www.andismith.com/blog/2011/11/on-and-off/ didn't work.
            • Without .live(), the jQuery script stops working after the BuildHistory list refresh
            • Jenkins core modification is new to me, so my change may not be following coding standards.

            Code:

            diff --git a/core/src/main/resources/hudson/widgets/BuildHistoryWidget/entries.jelly b/core/src/main/resources/hudson/widgets/BuildHistoryWidget/entries.jelly
            index e545724..4ffd3ac 100644
            --- a/core/src/main/resources/hudson/widgets/BuildHistoryWidget/entries.jelly
            +++ b/core/src/main/resources/hudson/widgets/BuildHistoryWidget/entries.jelly
            @@ -47,6 +47,7 @@ THE SOFTWARE.
                           <l:stopButton href="${rootURL}/queue/cancelItem?id=${item.id}" alt="${%cancel this build}"/>
                         </j:if>
                       </div>
            +          <div class="itemsummary" id="${id}">
                       <j:set var="cause" value="${item.getCauseOfBlockage()}"/>
                       <j:choose>
                         <j:when test="${cause!=null}">
                          (${%pending}—<st:include it="${cause}" page="summary.jelly"/>)
                        </j:when>
                        <j:otherwise>
                           (${%pending})
                         </j:otherwise>
                       </j:choose>
            -          ${item.params}
            +          </div>
            +          <div class="itemparams" id="${id}">${item.params}</div>
                     </td>
                   </tr>
                 </j:forEach>
            
            diff --git a/core/src/main/resources/lib/layout/layout.jelly b/core/src/main/resources/lib/layout/layout.jelly
            index a8466f3..6168fb9 100644
            --- a/core/src/main/resources/lib/layout/layout.jelly
            +++ b/core/src/main/resources/lib/layout/layout.jelly
            @@ -141,6 +140,11 @@ ${h.initPageVariables(context)}
                 <j:forEach var="pd" items="${h.pageDecorators}">
                   <st:include it="${pd}" page="header.jelly" optional="true" />
                 </j:forEach>
            +
            +    <!-- itemparams == Pending Job Build Parameters -->
            +    <style type="text/css"> div.itemparams { display:none; overflow: hidden; text-overflow: ellipsis; } </style>
            +    <script>jQuery(document).ready(function() { jQuery('div.itemsummary').live( "click", function() { jQuery('#'+this.id+'.itemparams').toggle(); }); });</script>
            +
               </head>
               <body id="jenkins jenkins-${h.version}" class="yui-skin-sam">
                 <!-- for accessibility, skip the entire navigation bar and etc and go straight to the head of the content -->
            
            jakub Jakub Czaplicki added a comment - - edited I did some work on this. The idea is to hide the build parameters and only show them when a user clicks on the "pending—Build #..." string, with ability to hide the long string again (and later with ability to view/expand all hidden build parameters). The working example script of what I aim for is here: http://jsfiddle.net/jakubczaplicki/WT88W/ It required modification of two files (see diffs below). I'd like a feedback and some suggestions/help on how to improve this code, such that it can be applied to master branch. Some notes: Tested on Jenkins ver. 1.565 Uses jQuery 1.7.2 (which is AFAIK default jQuery version built in jenkins) Every N seconds the BuildHistory is being refreshed. After each refresh, the parameters are hidden again. I am aware that I used deprecated jQuery's .live() method, however the suggestion on http://www.andismith.com/blog/2011/11/on-and-off/ didn't work. Without .live(), the jQuery script stops working after the BuildHistory list refresh Jenkins core modification is new to me, so my change may not be following coding standards. Code: diff --git a/core/src/main/resources/hudson/widgets/BuildHistoryWidget/entries.jelly b/core/src/main/resources/hudson/widgets/BuildHistoryWidget/entries.jelly index e545724..4ffd3ac 100644 --- a/core/src/main/resources/hudson/widgets/BuildHistoryWidget/entries.jelly +++ b/core/src/main/resources/hudson/widgets/BuildHistoryWidget/entries.jelly @@ -47,6 +47,7 @@ THE SOFTWARE. <l:stopButton href= "${rootURL}/queue/cancelItem?id=${item.id}" alt= "${%cancel this build}" /> </j: if > </div> + <div class= "itemsummary" id= "${id}" > <j:set var = "cause" value= "${item.getCauseOfBlockage()}" /> <j:choose> <j:when test= "${cause!= null }" > (${%pending}—<st:include it= "${cause}" page= "summary.jelly" />) </j:when> <j:otherwise> (${%pending}) </j:otherwise> </j:choose> - ${item.params} + </div> + <div class= "itemparams" id= "${id}" >${item.params}</div> </td> </tr> </j:forEach> diff --git a/core/src/main/resources/lib/layout/layout.jelly b/core/src/main/resources/lib/layout/layout.jelly index a8466f3..6168fb9 100644 --- a/core/src/main/resources/lib/layout/layout.jelly +++ b/core/src/main/resources/lib/layout/layout.jelly @@ -141,6 +140,11 @@ ${h.initPageVariables(context)} <j:forEach var = "pd" items= "${h.pageDecorators}" > <st:include it= "${pd}" page= "header.jelly" optional= " true " /> </j:forEach> + + <!-- itemparams == Pending Job Build Parameters --> + <style type= "text/css" > div.itemparams { display:none; overflow: hidden; text-overflow: ellipsis; } </style> + <script>jQuery(document).ready(function() { jQuery( 'div.itemsummary' ).live( "click" , function() { jQuery( '#' + this .id+ '.itemparams' ).toggle(); }); });</script> + </head> <body id= "jenkins jenkins-${h.version}" class= "yui-skin-sam" > <!-- for accessibility, skip the entire navigation bar and etc and go straight to the head of the content -->
            rin_ne rin_ne added a comment -

            Regarding live(), you should add event to any parent element of "div.itemsummary" using on(type, selector, function).

            "selector" is "div.itemsummary".

            rin_ne rin_ne added a comment - Regarding live(), you should add event to any parent element of "div.itemsummary" using on(type, selector, function). "selector" is "div.itemsummary".

            Back to drawing board. Summary from https://github.com/jenkinsci/jenkins/pull/1273 :

            • There's no jQuery available in the core
            • Should use Behaviour.add from within BuildHistoryWidget/entries.jelly rather than modifying the generic layout page.
            • gerrit-trigger-plugin ought to define custom parameter types, with customized presentation, rather than overloading standard textual ones
            jakub Jakub Czaplicki added a comment - Back to drawing board. Summary from https://github.com/jenkinsci/jenkins/pull/1273 : There's no jQuery available in the core Should use Behaviour.add from within BuildHistoryWidget/entries.jelly rather than modifying the generic layout page. gerrit-trigger-plugin ought to define custom parameter types, with customized presentation, rather than overloading standard textual ones
            danielbeck Daniel Beck added a comment -

            Really more of a Gerrit Trigger plugin issue – it seems everyone who's complained so far about the presentation used that plugin. It really should define its own parameter types with their own presentation.

            danielbeck Daniel Beck added a comment - Really more of a Gerrit Trigger plugin issue – it seems everyone who's complained so far about the presentation used that plugin. It really should define its own parameter types with their own presentation.
            rin_ne rin_ne added a comment - - edited

            A option that stores multiline text with no base64 encoded is already added by the below PR.
            https://github.com/jenkinsci/gerrit-trigger-plugin/pull/141

            So now gerrit trigger plugin has no issue for this.

            rin_ne rin_ne added a comment - - edited A option that stores multiline text with no base64 encoded is already added by the below PR. https://github.com/jenkinsci/gerrit-trigger-plugin/pull/141 So now gerrit trigger plugin has no issue for this.
            rin_ne rin_ne added a comment -

            Please read comments from the beginning.

            This issue is caused by the number of parameters rather than the size in a parameter value.
            Also it does not depend on whether parameter value is human readable or not.

            So I think that this issue would not be solved even if gerrit trigger has a custom parameter type.

            You should create a new ticket if you think that it is wrong to have parameter with encoded string value.

            rin_ne rin_ne added a comment - Please read comments from the beginning. This issue is caused by the number of parameters rather than the size in a parameter value. Also it does not depend on whether parameter value is human readable or not. So I think that this issue would not be solved even if gerrit trigger has a custom parameter type. You should create a new ticket if you think that it is wrong to have parameter with encoded string value.
            danielbeck Daniel Beck added a comment -

            rin_ne: Re my newest comment to the Github core PR: I checked the Jenkins instance with jobs with several parameters. The current implementation really isn't that easy to use (and looks worse) even with jobs that are not Gerrit-triggered (and in fact have quite short parameter values on average). So I'm OK with a 'hide by default' change to the widget.

            danielbeck Daniel Beck added a comment - rin_ne : Re my newest comment to the Github core PR: I checked the Jenkins instance with jobs with several parameters. The current implementation really isn't that easy to use (and looks worse) even with jobs that are not Gerrit-triggered (and in fact have quite short parameter values on average). So I'm OK with a 'hide by default' change to the widget.
            inb Ian Norton added a comment -

            This has started to annoy us after upgrading to 1.585. Previously the queued items would be quite tight/aggressivly wrapped and although I would have a rather "long" item in the queue it would not expand the width of the build queue panel.

            Since 1.585 however it tends to expand to the width so much that I have hardly any of the rest of the page left. This isn't a problem with too many or too wide parameters it is a problem with the UI rendering of this bit of HTML.

            inb Ian Norton added a comment - This has started to annoy us after upgrading to 1.585. Previously the queued items would be quite tight/aggressivly wrapped and although I would have a rather "long" item in the queue it would not expand the width of the build queue panel. Since 1.585 however it tends to expand to the width so much that I have hardly any of the rest of the page left. This isn't a problem with too many or too wide parameters it is a problem with the UI rendering of this bit of HTML.
            inb Ian Norton added a comment -

            The older appearance (ie not floating over the whole page) is restored by using this CSS:

            .transitive td {
             max-width: 2in;
             overflow: hidden;
             word-wrap: break-word;
            }
            
            inb Ian Norton added a comment - The older appearance (ie not floating over the whole page) is restored by using this CSS: .transitive td { max-width: 2in; overflow: hidden; word-wrap: break -word; }
            danielbeck Daniel Beck added a comment -

            Would something like the attached work? I moved the parameters into a tooltip (on the 'notepad' icon).

            For comparison, another queued build that was queued after build parameters were removed from the job, so no icon there.

            (I also removed the expected build number because of JENKINS-12827 etc., but that is unrelated to this issue.)

            danielbeck Daniel Beck added a comment - Would something like the attached work? I moved the parameters into a tooltip (on the 'notepad' icon). For comparison, another queued build that was queued after build parameters were removed from the job, so no icon there. (I also removed the expected build number because of JENKINS-12827 etc., but that is unrelated to this issue.)
            jkugler Joshua Kugler added a comment -

            @Ian Norton:

            Where is that CSS patch applied?

            jkugler Joshua Kugler added a comment - @Ian Norton: Where is that CSS patch applied?
            tfennelly Tom FENNELLY added a comment -

            I am working on something that should help resolve this issue: https://github.com/jenkinsci/jenkins/pull/1466

            CSS on it's own will not fix this in a cross browser way (word-wrap is not supported everywhere). For that reason, I ended up adding a bit of javascript that inserts breaks (zero-width spaces) in long non-whitespace char sequences. The history is still going to look nuts if there are crazy long build names (as in the screenshot on this ticket), but at least the history table should maintain it's structure.

            tfennelly Tom FENNELLY added a comment - I am working on something that should help resolve this issue: https://github.com/jenkinsci/jenkins/pull/1466 CSS on it's own will not fix this in a cross browser way (word-wrap is not supported everywhere). For that reason, I ended up adding a bit of javascript that inserts breaks (zero-width spaces) in long non-whitespace char sequences. The history is still going to look nuts if there are crazy long build names (as in the screenshot on this ticket), but at least the history table should maintain it's structure.
            tfennelly Tom FENNELLY added a comment -

            @Daniel I'm sure there'll be someone out there that "like" the params being always visible, but I think adding them as a tooltip makes sense for the vast majority of people.

            tfennelly Tom FENNELLY added a comment - @Daniel I'm sure there'll be someone out there that "like" the params being always visible, but I think adding them as a tooltip makes sense for the vast majority of people.
            vladichko Vlad Aginsky added a comment -

            Hi all, this is quite disturbing,
            I see https://github.com/jenkinsci/jenkins/pull/1470 (actual pull request) is merged, is it released already?

            vladichko Vlad Aginsky added a comment - Hi all, this is quite disturbing, I see https://github.com/jenkinsci/jenkins/pull/1470 (actual pull request) is merged, is it released already?
            danielbeck Daniel Beck added a comment -

            Released in 1.593.

            danielbeck Daniel Beck added a comment - Released in 1.593.

            Code changed in jenkins
            User: Daniel Beck
            Path:
            core/src/main/resources/hudson/widgets/BuildHistoryWidget/entries.jelly
            http://jenkins-ci.org/commit/jenkins/44e6b5d76211bc40e1a608ae3aa93c083befaeba
            Log:
            [FIXED JENKINS-22311] Show queue item parameters in tool tip

            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Daniel Beck Path: core/src/main/resources/hudson/widgets/BuildHistoryWidget/entries.jelly http://jenkins-ci.org/commit/jenkins/44e6b5d76211bc40e1a608ae3aa93c083befaeba Log: [FIXED JENKINS-22311] Show queue item parameters in tool tip

            Code changed in jenkins
            User: Jesse Glick
            Path:
            changelog.html
            core/src/main/resources/hudson/widgets/BuildHistoryWidget/entries.jelly
            http://jenkins-ci.org/commit/jenkins/7ef3d0bbf0f5250ac28797193eb105a06454d904
            Log:
            JENKINS-22311 Noting merge of #1492.

            Compare: https://github.com/jenkinsci/jenkins/compare/8e5199e377c7...7ef3d0bbf0f5

            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: changelog.html core/src/main/resources/hudson/widgets/BuildHistoryWidget/entries.jelly http://jenkins-ci.org/commit/jenkins/7ef3d0bbf0f5250ac28797193eb105a06454d904 Log: JENKINS-22311 Noting merge of #1492. Compare: https://github.com/jenkinsci/jenkins/compare/8e5199e377c7...7ef3d0bbf0f5
            dogfood dogfood added a comment -

            Integrated in jenkins_main_trunk #3924
            [FIXED JENKINS-22311] Show queue item parameters in tool tip (Revision 44e6b5d76211bc40e1a608ae3aa93c083befaeba)

            Result = SUCCESS
            daniel-beck : 44e6b5d76211bc40e1a608ae3aa93c083befaeba
            Files :

            • core/src/main/resources/hudson/widgets/BuildHistoryWidget/entries.jelly
            dogfood dogfood added a comment - Integrated in jenkins_main_trunk #3924 [FIXED JENKINS-22311] Show queue item parameters in tool tip (Revision 44e6b5d76211bc40e1a608ae3aa93c083befaeba) Result = SUCCESS daniel-beck : 44e6b5d76211bc40e1a608ae3aa93c083befaeba Files : core/src/main/resources/hudson/widgets/BuildHistoryWidget/entries.jelly

            Code changed in jenkins
            User: Daniel Beck
            Path:
            core/src/main/resources/hudson/widgets/BuildHistoryWidget/entries.jelly
            http://jenkins-ci.org/commit/jenkins/56795ba1f30ea9f060c9bb1e302fcff9827dd947
            Log:
            [FIXED JENKINS-22311] Show queue item parameters in tool tip

            (cherry picked from commit 44e6b5d76211bc40e1a608ae3aa93c083befaeba)

            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Daniel Beck Path: core/src/main/resources/hudson/widgets/BuildHistoryWidget/entries.jelly http://jenkins-ci.org/commit/jenkins/56795ba1f30ea9f060c9bb1e302fcff9827dd947 Log: [FIXED JENKINS-22311] Show queue item parameters in tool tip (cherry picked from commit 44e6b5d76211bc40e1a608ae3aa93c083befaeba)
            dogfood dogfood added a comment -

            Integrated in jenkins_main_trunk #4292
            [FIXED JENKINS-22311] Show queue item parameters in tool tip (Revision 56795ba1f30ea9f060c9bb1e302fcff9827dd947)

            Result = UNSTABLE
            ogondza : 56795ba1f30ea9f060c9bb1e302fcff9827dd947
            Files :

            • core/src/main/resources/hudson/widgets/BuildHistoryWidget/entries.jelly
            dogfood dogfood added a comment - Integrated in jenkins_main_trunk #4292 [FIXED JENKINS-22311] Show queue item parameters in tool tip (Revision 56795ba1f30ea9f060c9bb1e302fcff9827dd947) Result = UNSTABLE ogondza : 56795ba1f30ea9f060c9bb1e302fcff9827dd947 Files : core/src/main/resources/hudson/widgets/BuildHistoryWidget/entries.jelly

            People

              danielbeck Daniel Beck
              eric_griswold Eric Griswold
              Votes:
              9 Vote for this issue
              Watchers:
              15 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: