feat(codepipeline): change to stand-alone Artifacts #2338

skinny85 · 2019-04-20T00:12:49Z

This commit changes the way Artifacts are used in CodePipeline.
Instead of being properties on the Actions,
they are now stand-alone objects,
created independently of the Actions,
and referenced when instantiating them.
This is to not force users to assign Actions to intermediate variables
when defining a Pipeline.

This change had a few interesting consequences:

We no longer needed the abstract subclasses of Action like SourceAction,
DeployAction, etc., and so they were removed.
The old naming convention of inputArtifacts and outputArtifactNames
was shortened to be simply inputs and outputs.
There was no longer any need to differentiate Build and Test Actions,
and so the two CodeBuild and Jenkins Actions were merged into one.

BREAKING CHANGE: CodePipeline Actions no longer have the outputArtifact
and outputArtifacts properties.

inputArtifact(s) and additionalInputArtifacts properties were renamed
to input(s) and extraInputs.
outputArtifactName(s) and additionalOutputArtifactNames properties
were renamed to output(s) and extraOutputs.
The classes CodeBuildBuildAction and CodeBuildTestAction were merged
into one class CodeBuildAction.
The classes JenkinsBuildAction and JenkinsTestAction were merged
into one class JenkinsAction.

Fixes #2230

Pull Request Checklist

Testing
- Unit test added (prefer not to modify an existing test, otherwise, it's probably a breaking change)
- CLI change?: coordinate update of integration tests with team
- cdk-init template change?: coordinated update of integration tests with team
Docs
- jsdocs: All public APIs documented
- README: README and/or documentation topic updated
Title and Description
- Change type: title prefixed with fix, feat will appear in changelog
- Title: use lower-case and doesn't end with a period
- Breaking?: last paragraph: "BREAKING CHANGE: <describe what changed + link for details>"
- Issues: Indicate issues fixed via: "Fixes #xxx" or "Closes #xxx"
Sensitive Modules (requires 2 PR approvers)
- IAM Policy Document (in @aws-cdk/aws-iam)
- EC2 Security Groups and ACLs (in @aws-cdk/aws-ec2)
- Grant APIs (only if not based on official documentation with a reference)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

packages/@aws-cdk/aws-codepipeline/lib/action.ts

eladb · 2019-04-22T17:15:41Z

packages/@aws-cdk/app-delivery/test/test.pipeline-deploy-stack-action.ts

@@ -305,7 +304,7 @@ class FakeAction extends codepipeline.Action {
  constructor(actionName: string) {
    super({
      actionName,
-      artifactBounds: codepipeline.defaultBounds(),
+      artifactBounds: { minInputs: 0, maxInputs: 5, minOutputs: 0, maxOutputs: 5 },


Why did we get rid of the defaultBounds helper?

It wasn't really useful, and I don't even think the name was accurate (having 0 to 5 inputs & outputs is definitely not a default for Actions).

packages/@aws-cdk/aws-codepipeline-actions/lib/codebuild/build-action.ts

packages/@aws-cdk/aws-codepipeline/lib/artifact.ts

eladb · 2019-04-22T17:52:25Z

packages/@aws-cdk/aws-codepipeline/lib/artifact.ts

+
+  private _artifactName?: string;
+
+  constructor(artifactName?: string) {


I guess make this private

Can't - it's used in Artifact.

?! It's used in the static method, no?

class Foo { public static makeFoo(): Foo { return new Foo() ; } private constructor() { } }

Now only Foo.makeFoo can be used to make foo and not new Foo().

Right, except the static method is not that simple in this case:

public static artifactPath(artifactName: string, fileName: string): ArtifactPath { return new ArtifactPath(Artifact.artifact(artifactName), fileName); }

So it's not simply equivalent to new ArtifactPath(artifact, fileName).

I am taking about the ctor of Artifact... I feel I am missing something... But nevermind, I don't really care

packages/@aws-cdk/aws-codepipeline/lib/artifact.ts

packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts

packages/@aws-cdk/aws-codepipeline/lib/stage.ts

skinny85 · 2019-04-22T20:19:50Z

Changes after review comments.

This commit changes the way Artifacts are used in CodePipeline. Instead of being properties on the Actions, they are now stand-alone objects, created independently of the Actions, and referenced when instantiating them. This is to not force users to assign Actions to intermediate variables when defining a Pipeline. This change had a few interesting consequences: * We no longer needed the abstract subclasses of Action like SourceAction, DeployAction, etc., and so they were removed. * The old naming convention of `inputArtifacts` and `outputArtifactNames` was shortened to be simply `inputs` and `outputs`. * There was no longer any need to differentiate Build and Test Actions, and so the two CodeBuild and Jenkins Actions were merged into one. BREAKING CHANGE: CodePipeline Actions no longer have the `outputArtifact` and `outputArtifacts` properties. * `inputArtifact(s)` and `additionalInputArtifacts` properties were renamed to `input(s)` and `extraInputs`. * `outputArtifactName(s)` and `additionalOutputArtifactNames` properties were renamed to `output(s)` and `extraOutputs`. * The classes `CodeBuildBuildAction` and `CodeBuildTestAction` were merged into one class `CodeBuildAction`. * The classes `JenkinsBuildAction` and `JenkinsTestAction` were merged into one class `JenkinsAction`.

skinny85 · 2019-04-22T20:57:32Z

Added a missing test for action Artifacts bound validation.

This commit changes the way Artifacts are used in CodePipeline. Instead of being properties on the Actions, they are now stand-alone objects, created independently of the Actions, and referenced when instantiating them. This is to not force users to assign Actions to intermediate variables when defining a Pipeline. This change had a few interesting consequences: * We no longer needed the abstract subclasses of Action like SourceAction, DeployAction, etc., and so they were removed. * The old naming convention of `inputArtifacts` and `outputArtifactNames` was shortened to be simply `inputs` and `outputs`. * There was no longer any need to differentiate Build and Test Actions, and so the two CodeBuild and Jenkins Actions were merged into one. BREAKING CHANGE: CodePipeline Actions no longer have the `outputArtifact` and `outputArtifacts` properties. * `inputArtifact(s)` and `additionalInputArtifacts` properties were renamed to `input(s)` and `extraInputs`. * `outputArtifactName(s)` and `additionalOutputArtifactNames` properties were renamed to `output(s)` and `extraOutputs`. * The classes `CodeBuildBuildAction` and `CodeBuildTestAction` were merged into one class `CodeBuildAction`. * The classes `JenkinsBuildAction` and `JenkinsTestAction` were merged into one class `JenkinsAction`.

skinny85 requested review from RomainMuller and a team as code owners April 20, 2019 00:12

skinny85 requested a review from eladb April 20, 2019 00:14

skinny85 force-pushed the feature/artifacts-in-codepipeline branch from 4c60f74 to 39b87e9 Compare April 20, 2019 00:28

aecollver reviewed Apr 22, 2019

View reviewed changes

packages/@aws-cdk/aws-codepipeline/lib/action.ts Outdated Show resolved Hide resolved

eladb approved these changes Apr 22, 2019

View reviewed changes

skinny85 force-pushed the feature/artifacts-in-codepipeline branch from 353ab4c to 4a40736 Compare April 22, 2019 20:19

skinny85 force-pushed the feature/artifacts-in-codepipeline branch from 4a40736 to 75e5e6c Compare April 22, 2019 20:57

skinny85 merged commit b778e10 into aws:master Apr 22, 2019

skinny85 deleted the feature/artifacts-in-codepipeline branch April 22, 2019 23:19

fulghum assigned skinny85 Apr 23, 2019

NGL321 added the contribution/core This is a PR that came from AWS. label Sep 27, 2019

This was referenced Dec 12, 2019

[Snyk] Upgrade @aws-cdk/cdk from 0.22.0 to 0.35.0 MechanicalRock/scdk#3

Closed

[Snyk] Upgrade @aws-cdk/cdk from 0.24.1 to 0.35.0 MechanicalRock/cdk-constructs#8

Open

[Snyk] Upgrade @aws-cdk/cdk from 0.24.1 to 0.35.0 MechanicalRock/account-reaper#6

Open

skinny85 mentioned this pull request Jan 2, 2020

feat(codepipeline): Pipeline Variables #5604

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(codepipeline): change to stand-alone Artifacts #2338

feat(codepipeline): change to stand-alone Artifacts #2338

skinny85 commented Apr 20, 2019 •

edited

Loading

eladb Apr 22, 2019

skinny85 Apr 22, 2019

eladb Apr 22, 2019

skinny85 Apr 22, 2019

eladb Apr 22, 2019

skinny85 Apr 22, 2019

eladb Apr 22, 2019

skinny85 commented Apr 22, 2019

skinny85 commented Apr 22, 2019


		private _artifactName?: string;

		constructor(artifactName?: string) {

feat(codepipeline): change to stand-alone Artifacts #2338

feat(codepipeline): change to stand-alone Artifacts #2338

Conversation

skinny85 commented Apr 20, 2019 • edited Loading

Pull Request Checklist

eladb Apr 22, 2019

Choose a reason for hiding this comment

skinny85 Apr 22, 2019

Choose a reason for hiding this comment

eladb Apr 22, 2019

Choose a reason for hiding this comment

skinny85 Apr 22, 2019

Choose a reason for hiding this comment

eladb Apr 22, 2019

Choose a reason for hiding this comment

skinny85 Apr 22, 2019

Choose a reason for hiding this comment

eladb Apr 22, 2019

Choose a reason for hiding this comment

skinny85 commented Apr 22, 2019

skinny85 commented Apr 22, 2019

skinny85 commented Apr 20, 2019 •

edited

Loading