[JIRA] (JENKINS-40573) JSON representation should only use named arguments

3 views
Skip to first unread message

kzantow@cloudbees.com (JIRA)

unread,
Dec 20, 2016, 10:49:01 AM12/20/16
to jenkinsc...@googlegroups.com
Keith Zantow created an issue
 
Jenkins / Improvement JENKINS-40573
JSON representation should only use named arguments
Issue Type: Improvement Improvement
Assignee: Andrew Bayer
Components: pipeline-model-definition-plugin
Created: 2016/Dec/20 3:48 PM
Priority: Major Major
Reporter: Keith Zantow

Currently, the JSON format has 2 different representations of step parameters, e.g.:

"steps": [{
    "name": "bat",
    "arguments": {
        "isLiteral": true,
        "value": "someBatScript"
    }
}]

vs.

"steps": [{
    "name": "bat",
    "arguments": [{
        "key": "script",
        "value": {
            "isLiteral": true,
            "value": "someBatScript"
        },
    }]
}]

Since JSON is intended only to support tools, this should be consolidated to only use named arguments and the conversion to/from the Jenkinsfile should handle converting to/from these "single required parameter" forms, giving the most idiomatic Jenkinsfile from a JSON representation wherever possible.

I think this should be fairly straightforward to implement by checking if the step has a single required parameter, to use that for conversion from a Jenkinsfile and if that is the only parameter supplied by the JSON, convert it back to the idiomatic form.

Add Comment Add Comment
 
This message was sent by Atlassian JIRA (v7.1.7#71011-sha1:2526d7c)
Atlassian logo

andrew.bayer@gmail.com (JIRA)

unread,
Jan 7, 2017, 7:16:01 PM1/7/17
to jenkinsc...@googlegroups.com
Andrew Bayer started work on Improvement JENKINS-40573
 
Change By: Andrew Bayer
Status: Open In Progress

andrew.bayer@gmail.com (JIRA)

unread,
Jan 7, 2017, 7:17:01 PM1/7/17
to jenkinsc...@googlegroups.com

andrew.bayer@gmail.com (JIRA)

unread,
Jan 7, 2017, 8:59:02 PM1/7/17
to jenkinsc...@googlegroups.com

Well, maybe it can't, exactly. Steps that solely take a closure with no other arguments where the closure contents aren't parsed further (like script most notably) aren't easy to translate into named arguments form. So unless I put in some truly bonkers hardcoding logic to handle translating that case into named arguments form and vice versa, I can't have all step "arguments" always be named arguments in the JSON representation.

At least I don't think so. Playing with it further now.

andrew.bayer@gmail.com (JIRA)

unread,
Jan 7, 2017, 9:46:01 PM1/7/17
to jenkinsc...@googlegroups.com

I really need to stop saying "I can't do X" 'cos whenever I do, I seem to figure out a potentially insane way to do X. Still verifying tests, but assuming they pass, I have a solution for this, albeit one that may be crazy.

kzantow@cloudbees.com (JIRA)

unread,
Jan 7, 2017, 9:51:01 PM1/7/17
to jenkinsc...@googlegroups.com

I was doing this in the UI - the logic should be the same, not really crazy, right? If there is exactly one parameter, and it is the one required parameter, just put it in the right form?

andrew.bayer@gmail.com (JIRA)

unread,
Jan 7, 2017, 9:54:01 PM1/7/17
to jenkinsc...@googlegroups.com

andrew.bayer@gmail.com (JIRA)

unread,
Jan 7, 2017, 9:55:01 PM1/7/17
to jenkinsc...@googlegroups.com

andrew.bayer@gmail.com (JIRA)

unread,
Jan 7, 2017, 10:00:02 PM1/7/17
to jenkinsc...@googlegroups.com

It's the special-casing for script (and when expressions) that get whacky - take a look at the PR, lemme know if I got too whacky. =)

andrew.bayer@gmail.com (JIRA)

unread,
Jan 7, 2017, 10:01:01 PM1/7/17
to jenkinsc...@googlegroups.com

I also retained support for the single-argument approach, 'cos, well, I felt like it. And I'm not 100% sure we don't end up doing something similar with the argument lists elsewhere where we would want to keep the single-argument approach.

scm_issue_link@java.net (JIRA)

unread,
Jan 10, 2017, 10:53:04 AM1/10/17
to jenkinsc...@googlegroups.com

Code changed in jenkins
User: Andrew Bayer
Path:
pipeline-model-api/pom.xml
pipeline-model-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/DescriptorLookupCache.java
pipeline-model-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/AbstractModelASTCodeBlock.java
pipeline-model-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTNamedArgumentList.java
pipeline-model-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTStep.java
pipeline-model-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTTreeStep.java
pipeline-model-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidator.java
pipeline-model-declarative-agent/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/agent/DeclarativeAgentDescriptor.java
pipeline-model-definition/pom.xml
pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ModelParser.groovy
pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidatorImpl.groovy
pipeline-model-definition/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/agent/impl/Any.java
pipeline-model-definition/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/agent/impl/Label.java
pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/BasicModelDefTest.java
pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/endpoints/ModelConverterActionStepsTest.java
pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/validator/JSONValidationTest.java
pipeline-model-definition/src/test/resources/json/agentAny.json
pipeline-model-definition/src/test/resources/json/agentDocker.json
pipeline-model-definition/src/test/resources/json/agentLabel.json
pipeline-model-definition/src/test/resources/json/agentNoneWithNode.json
pipeline-model-definition/src/test/resources/json/agentTypeOrdering.json
pipeline-model-definition/src/test/resources/json/basicWhen.json
pipeline-model-definition/src/test/resources/json/environmentInStage.json
pipeline-model-definition/src/test/resources/json/errors/agentMissingRequiredParam.json
pipeline-model-definition/src/test/resources/json/errors/agentUnknownParamForType.json
pipeline-model-definition/src/test/resources/json/errors/emptyEnvironment.json
pipeline-model-definition/src/test/resources/json/errors/emptyJobProperties.json
pipeline-model-definition/src/test/resources/json/errors/emptyParameters.json
pipeline-model-definition/src/test/resources/json/errors/emptyPostBuild.json
pipeline-model-definition/src/test/resources/json/errors/emptyTriggers.json
pipeline-model-definition/src/test/resources/json/errors/invalidBuildCondition.json
pipeline-model-definition/src/test/resources/json/errors/invalidParameterTypeMethodCall.json
pipeline-model-definition/src/test/resources/json/errors/invalidWrapperType.json
pipeline-model-definition/src/test/resources/json/errors/missingAgent.json
pipeline-model-definition/src/test/resources/json/errors/missingRequiredMethodCallArg.json
pipeline-model-definition/src/test/resources/json/errors/missingRequiredStepParameters.json
pipeline-model-definition/src/test/resources/json/errors/missingStages.json
pipeline-model-definition/src/test/resources/json/errors/mixedMethodArgs.json
pipeline-model-definition/src/test/resources/json/errors/notInstalledToolType.json
pipeline-model-definition/src/test/resources/json/errors/notInstalledToolVersion.json
pipeline-model-definition/src/test/resources/json/errors/notificationsSectionRemoved.json
pipeline-model-definition/src/test/resources/json/errors/perStageConfigUnknownSection.json
pipeline-model-definition/src/test/resources/json/errors/rejectParallelMixedInSteps.json
pipeline-model-definition/src/test/resources/json/errors/rejectPropertiesStepInMethodCall.json
pipeline-model-definition/src/test/resources/json/errors/rejectStageInSteps.json
pipeline-model-definition/src/test/resources/json/errors/stageWithoutName.json
pipeline-model-definition/src/test/resources/json/errors/unknownAgentType.json
pipeline-model-definition/src/test/resources/json/errors/unknownBareAgentType.json
pipeline-model-definition/src/test/resources/json/errors/unknownStepParameter.json
pipeline-model-definition/src/test/resources/json/errors/unlistedToolType.json
pipeline-model-definition/src/test/resources/json/errors/wrongParameterNameMethodCall.json
pipeline-model-definition/src/test/resources/json/globalLibrarySuccess.json
pipeline-model-definition/src/test/resources/json/legacyMetaStepSyntax.json
pipeline-model-definition/src/test/resources/json/metaStepSyntax.json
pipeline-model-definition/src/test/resources/json/multipleVariablesForAgent.json
pipeline-model-definition/src/test/resources/json/multipleWrappers.json
pipeline-model-definition/src/test/resources/json/parallelPipeline.json
pipeline-model-definition/src/test/resources/json/parallelPipelineQuoteEscaping.json
pipeline-model-definition/src/test/resources/json/parallelPipelineWithFailFast.json
pipeline-model-definition/src/test/resources/json/parallelPipelineWithSpaceInBranch.json
pipeline-model-definition/src/test/resources/json/perStageConfigAgent.json
pipeline-model-definition/src/test/resources/json/simpleEnvironment.json
pipeline-model-definition/src/test/resources/json/simpleJobProperties.json
pipeline-model-definition/src/test/resources/json/simpleParameters.json
pipeline-model-definition/src/test/resources/json/simplePipeline.json
pipeline-model-definition/src/test/resources/json/simplePostBuild.json
pipeline-model-definition/src/test/resources/json/simpleScript.json
pipeline-model-definition/src/test/resources/json/simpleTools.json
pipeline-model-definition/src/test/resources/json/simpleTriggers.json
pipeline-model-definition/src/test/resources/json/simpleWrapper.json
pipeline-model-definition/src/test/resources/json/skippedWhen.json
pipeline-model-definition/src/test/resources/json/steps/arrayEcho.json
pipeline-model-definition/src/test/resources/json/steps/simpleEcho.json
pipeline-model-definition/src/test/resources/json/steps/simpleScript.json
pipeline-model-definition/src/test/resources/json/stringsNeedingEscapeLogic.json
pipeline-model-definition/src/test/resources/json/toolsInStage.json
pipeline-model-definition/src/test/resources/json/twoStagePipeline.json
pipeline-model-definition/src/test/resources/json/validStepParameters.json
pipeline-model-definition/src/test/resources/json/whenBranch.json
pipeline-model-definition/src/test/resources/json/whenEnv.json
http://jenkins-ci.org/commit/pipeline-model-definition-plugin/67fff2472d52e9d989dd1f9e124c31039fda21a3
Log:
[FIXED JENKINS-40573] Re-do step arguments to always be named in JSON representation.

While still serializing to Groovy as, e.g., "echo('foo')" when
appropriate (i.e., sole required parameter, just one parameter
specified, no closure involved).

andrew.bayer@gmail.com (JIRA)

unread,
Jan 10, 2017, 10:53:05 AM1/10/17
to jenkinsc...@googlegroups.com

bitwiseman@gmail.com (JIRA)

unread,
Oct 22, 2019, 11:24:48 PM10/22/19
to jenkinsc...@googlegroups.com
Liam Newman closed an issue as Fixed
 

Bulk closing resolved issues.

Change By: Liam Newman
Status: Resolved Closed
This message was sent by Atlassian Jira (v7.13.6#713006-sha1:cc4451f)
Atlassian logo
Reply all
Reply to author
Forward
0 new messages