[JENKINS-52491] - PipelineGenerator extensibility and support of custom sections #25

oleg-nenashev · 2018-08-02T14:40:42Z

This is a result of the screenshare with @gautamabhishek46 and @martinda . We created a base extension engine and added support of custom pipeline sections defined as extensions. Also, some refactoring

- PipelineGenerator extension point
- Implementation for agent
- Custom sections support
- Detach all other generator code to extensions
- Add annotations in methods
- Fix the exception propagation, use the new type
- Add extra unit tests

kwhetstone

Overall, this looks great. Is the plan to do all the conversions in this ticket, or to start out with these changes and then do the other elements in a second PR?

kwhetstone · 2018-08-02T16:45:13Z

src/main/java/io/jenkins/plugins/sprp/models/YamlPipeline.java

@@ -116,4 +122,13 @@ public void setPost(Post post) {
    public void setSteps(ArrayList<LinkedHashMap<String, Object>> steps) {
        this.steps = Stage.generateSteps(steps);
    }
+
+    public void setSections(@CheckForNull ArrayList<CustomPipelineSection> sections) {


Would it make sense to have an addSection method?

kwhetstone · 2018-08-02T16:50:47Z

src/main/java/io/jenkins/plugins/sprp/PipelineGenerator.java

@@ -399,7 +387,7 @@ private Sequence doMappingForSequence(List<Object> objects) throws NotSupportedE
        return snippetLines;
    }

-    public String autoAddTabs(ArrayList<String> snippetLines) {
+    public static String autoAddTabs(ArrayList<String> snippetLines) {


Random implementation question: do we need to to call this outside of displaying the pipeline to end users? Declarative does a fairly good job surrounding things with curly braces. Does this become more of a "prettyPrint" detail?

kwhetstone · 2018-08-02T17:01:42Z

src/main/java/io/jenkins/plugins/sprp/impl/AgentGenerator.java

+            agentLines.add("agent any");
+        } else if (agent.getAnyOrNone() != null)
+            agentLines.add("agent " + agent.getAnyOrNone());
+        else if(agent.isNone()) {


I'm still not sure why we have 2 checks here. getAnyOrNone() literally returns any or none. We should never reach this last check.

kwhetstone · 2018-08-02T17:05:12Z

src/main/java/io/jenkins/plugins/sprp/impl/AgentGenerator.java

+        return object instanceof Agent;
+    }
+
+    private List<String> getCommonOptionsOfAgent(Agent agent) {


For getting this to work for now, this is good. I think there's a way to get this from the extension point this matches. I'd have to dig though; you could probably even ask on irc.

(cherry picked from commit 432f56c)

oleg-nenashev · 2018-08-06T21:18:23Z

There are still some bugs to be fixes:

[INFO] Total bugs: 3

[INFO] gitConfig must be non-null but is marked as nullable [io.jenkins.plugins.sprp.generators.GitPushStageGenerator] At GitPushStageGenerator.java:[lines 21-32] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE

[INFO] reportsAndArtifactsInfo must be non-null but is marked as nullable [io.jenkins.plugins.sprp.generators.PublishReportsAndArtifactsStageGenerator] At PublishReportsAndArtifactsStageGenerator.java:[lines 24-54] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE

[INFO] stage must be non-null but is marked as nullable [io.jenkins.plugins.sprp.generators.StageGenerator] At StageGenerator.java:[lines 29-54] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE

[INFO]

kwhetstone · 2018-08-07T18:19:45Z

src/main/java/io/jenkins/plugins/sprp/YamlToPipeline.java

@@ -39,37 +51,51 @@ public String generatePipeline(InputStream yamlScriptInputStream, GitConfig gitC
            scriptLines.add("stage('Build') {");


Random question: would it be possible to identify that there are only steps defined earlier and to create a Stage with those steps, assigning it to a value in yamlPipeline therefore eliminating these 2 implementation paths? Or is that more trouble than it's worth?

kwhetstone · 2018-08-07T18:20:46Z

src/main/java/io/jenkins/plugins/sprp/YamlToPipeline.java

            }
        }

        // Archive artifacts stage
-        scriptLines.addAll(psg.getArchiveArtifactsStage(yamlPipeline.getArchiveArtifacts()));
+        scriptLines.addAll(PipelineGenerator.convert("archiveArtifactStage", yamlPipeline.getArchiveArtifacts()));


Should this really continue to act different than the other stages? Isn't it just another stage at this point?

kwhetstone · 2018-08-07T18:30:14Z

Just a few general comments about the code that could be addressed in another PR. There's a lot going on here. I do really appreciate all the tests

gautamabhishek46 · 2018-08-09T16:58:05Z

Not sure how to remove the findbugs error.

oleg-nenashev

When you use @SuppressFBWarnings("RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE"), it pretty much always means suppression of a real issue in the code

oleg-nenashev

Test code is still improperly handled/annotated by FB warnings, but it is not a big deal. IMO it is ready to go

oleg-nenashev added 3 commits August 2, 2018 16:09

[JENKINS-52491] - Create base engine for Generator extensibility

b2c1f7a

Add annotations to Git Push configuration in API

8c185a6

[JENKINS-52491] - Demo the section extensibility engine

bcc89c0

oleg-nenashev requested review from gautamabhishek46 and martinda August 2, 2018 14:40

kwhetstone reviewed Aug 2, 2018

View reviewed changes

gautamabhishek46 added 2 commits August 6, 2018 01:45

All functions in PipelineGenerator class are detached to extensions.

e57258c

(cherry picked from commit 432f56c)

Findbugs error resolved

311bde3

martinda mentioned this pull request Aug 7, 2018

Extension point #24

Closed

kwhetstone reviewed Aug 7, 2018

View reviewed changes

Throwing ConversionException only, findbugs warnings suppressed

211c016

oleg-nenashev commented Aug 9, 2018

View reviewed changes

Findbugs errors reolved

2f514b2

oleg-nenashev commented Aug 11, 2018

View reviewed changes

gautamabhishek46 merged commit 753acc6 into master Aug 12, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-52491] - PipelineGenerator extensibility and support of custom sections #25

[JENKINS-52491] - PipelineGenerator extensibility and support of custom sections #25

oleg-nenashev commented Aug 2, 2018 •

edited by gautamabhishek46

kwhetstone left a comment

kwhetstone Aug 2, 2018

kwhetstone Aug 2, 2018

kwhetstone Aug 2, 2018

kwhetstone Aug 2, 2018

oleg-nenashev commented Aug 6, 2018

kwhetstone Aug 7, 2018

kwhetstone Aug 7, 2018

kwhetstone commented Aug 7, 2018

gautamabhishek46 commented Aug 9, 2018

oleg-nenashev left a comment

oleg-nenashev left a comment

		@@ -39,37 +51,51 @@ public String generatePipeline(InputStream yamlScriptInputStream, GitConfig gitC
		scriptLines.add("stage('Build') {");

[JENKINS-52491] - PipelineGenerator extensibility and support of custom sections #25

[JENKINS-52491] - PipelineGenerator extensibility and support of custom sections #25

Conversation

oleg-nenashev commented Aug 2, 2018 • edited by gautamabhishek46

kwhetstone left a comment

Choose a reason for hiding this comment

kwhetstone Aug 2, 2018

Choose a reason for hiding this comment

kwhetstone Aug 2, 2018

Choose a reason for hiding this comment

kwhetstone Aug 2, 2018

Choose a reason for hiding this comment

kwhetstone Aug 2, 2018

Choose a reason for hiding this comment

oleg-nenashev commented Aug 6, 2018

kwhetstone Aug 7, 2018

Choose a reason for hiding this comment

kwhetstone Aug 7, 2018

Choose a reason for hiding this comment

kwhetstone commented Aug 7, 2018

gautamabhishek46 commented Aug 9, 2018

oleg-nenashev left a comment

Choose a reason for hiding this comment

oleg-nenashev left a comment

Choose a reason for hiding this comment

oleg-nenashev commented Aug 2, 2018 •

edited by gautamabhishek46