feat(scheduler-targets): add CodePipeline as target for scheduler by daschaa · Pull Request #27799 · aws/aws-cdk

daschaa · 2023-11-01T19:12:14Z

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

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

lpizzinidev

Thanks, looks good! 💪
Just an adjustment on region and account values for imported pipelines.

packages/@aws-cdk/aws-scheduler-targets-alpha/lib/codepipeline-start-pipeline-execution.ts

daschaa · 2023-11-04T16:15:25Z

Thanks, looks good! 💪

Just an adjustment on region and account values for imported pipelines.

I have created an issue for that: #27816

daschaa · 2023-11-05T11:26:55Z

@lpizzinidev Do you think I should "fix" the fromPipelineArn function in this PR or do this seperately in #27816 ?
The issue still under investigating, so i'm not sure if the implementation in fromPipelineArn is intended?

What are your thoughts on that?

lpizzinidev · 2023-11-05T15:42:48Z

@daschaa
I encountered the same problem while implementing #27837 for SnsPublish.

In my opinion, this is a bug on imported resources from ARNs, as you state in #27816 and, for the CodePipeline target, it should be fixed in this PR since it's directly related to the implementation.

However, it's also good to have an issue open to keep track of the "bug", as it does affect multiple resources and some discussion might be needed to get to an ideal and consistent solution.

aws-cdk-automation · 2023-11-22T00:09:27Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2023-11-23T00:09:13Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2023-11-24T00:08:55Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2023-11-25T00:08:50Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2023-11-26T00:09:54Z

This PR has been in the MERGE CONFLICTS state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

lpizzinidev

Looks good, thanks!
Just some tests following the last changes are required in my opinion.

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

lpizzinidev · 2023-12-07T16:52:35Z

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

+      const stack = new cdk.Stack(new cdk.App(), 'Stack', {
+        env: {
+          region: 'eu-west-1',
+          account: '234567890123',
+        },
+      });
+
+      const pipeline = codepipeline.Pipeline.fromPipelineArn(stack, 'Pipeline',
+        'arn:aws:codepipeline:us-east-1:123456789012:MyPipeline');
+
+      expect(pipeline.env.region).toEqual('us-east-1');
+      expect(pipeline.env.account).toEqual('123456789012');


Suggested change

const stack = new cdk.Stack(new cdk.App(), 'Stack', {

env: {

region: 'eu-west-1',

account: '234567890123',

},

});

const pipeline = codepipeline.Pipeline.fromPipelineArn(stack, 'Pipeline',

'arn:aws:codepipeline:us-east-1:123456789012:MyPipeline');

expect(pipeline.env.region).toEqual('us-east-1');

expect(pipeline.env.account).toEqual('123456789012');

const stack = new cdk.Stack();

const pipeline = codepipeline.Pipeline.fromPipelineArn(stack, 'Pipeline',

'arn:aws:codepipeline:us-west-1:123456789012:MyPipeline');

expect(pipeline.env.region).toEqual('us-west-1');

expect(pipeline.env.account).toEqual('123456789012');

Let's keep it simple.

I prefer adding the env to the stack explicitly, because it shows that the import really does not use the environment of the stack 🤔

aws-cdk-automation · 2023-12-11T00:09:01Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2023-12-12T00:08:46Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2023-12-13T00:08:31Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2023-12-14T00:08:44Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2023-12-15T00:08:53Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2023-12-16T00:08:46Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

kaizencc

Thanks @daschaa

kaizencc · 2023-12-18T16:47:18Z

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

-    return new Import(scope, id);
+    return new Import(scope, id, {
+      environmentFromArn: pipelineArn,
+    });


This is just a bug fix to codepipeline that we should have had in a long time ago lol

mergify · 2023-12-18T16:48:41Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify · 2023-12-18T16:52:24Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

Pull request has been modified.

mergify · 2023-12-18T18:46:38Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2023-12-18T19:14:32Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 6f514a8
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2023-12-18T19:15:43Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

🎉 Creates draft for codepipeline

77af2da

github-actions bot added the star-contributor [Pilot] contributed between 25-49 PRs to the CDK label Nov 1, 2023

aws-cdk-automation requested a review from a team November 1, 2023 19:12

github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 labels Nov 1, 2023

aws-cdk-automation previously requested changes Nov 1, 2023

View reviewed changes

daschaa added 2 commits November 4, 2023 08:37

🧪 Adds tests for codepipeline target

dc168d0

Merge branch 'main' into 27449_codepipeline

70b76e4

daschaa changed the title ~~feat(scheduler-targets-alpha): add CodePipeline as target for scheduler~~ feat(scheduler-targets): add CodePipeline as target for scheduler Nov 4, 2023

daschaa added 2 commits November 4, 2023 08:43

📝 Adds README entry

6c8820d

📝 Adds docstring to codepipelinestartpipelineexecution

592a94e

daschaa marked this pull request as ready for review November 4, 2023 07:46

daschaa added 2 commits November 4, 2023 09:28

🐛 Fixes import

2a7ffce

🐛 Fixes default value for props

00d0aa8

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Nov 4, 2023

lpizzinidev reviewed Nov 4, 2023

View reviewed changes

packages/@aws-cdk/aws-scheduler-targets-alpha/lib/codepipeline-start-pipeline-execution.ts Outdated Show resolved Hide resolved

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Nov 4, 2023

daschaa mentioned this pull request Nov 6, 2023

(codepipeline): Imported pipeline has incorrect account id #27816

Closed

daschaa added 2 commits November 26, 2023 08:32

🐛 Adds region and account import from arn

e1745a3

Merge branch 'main' into 27449_codepipeline

5e70182

daschaa added 2 commits December 6, 2023 11:45

🔥 Removes unnecessary imports

cdb897a

🚨 Fixes linter warnings

0001174

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Dec 6, 2023

lpizzinidev suggested changes Dec 6, 2023

View reviewed changes

packages/aws-cdk-lib/aws-codepipeline/lib/pipeline.ts Show resolved Hide resolved

daschaa added 3 commits December 7, 2023 10:12

🧪 Adds test for import vpc

f48d2a9

🔒️ Fixes issue with wrong account id

3481fd8

🐛 Fixes account number

b2249bd

lpizzinidev suggested changes Dec 7, 2023

View reviewed changes

lpizzinidev approved these changes Dec 8, 2023

View reviewed changes

Merge branch 'main' into 27449_codepipeline

100b4af

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 16, 2023

kaizencc previously approved these changes Dec 18, 2023

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 18, 2023

Merge branch 'main' into 27449_codepipeline

1734baa

kaizencc approved these changes Dec 18, 2023

View reviewed changes

Merge branch 'main' into 27449_codepipeline

6f514a8

mergify bot merged commit 8c44f32 into aws:main Dec 18, 2023

daschaa deleted the 27449_codepipeline branch May 7, 2024 06:47

Conversation

daschaa commented Nov 1, 2023

Uh oh!

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Uh oh!

lpizzinidev left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

daschaa commented Nov 4, 2023

Uh oh!

daschaa commented Nov 5, 2023

Uh oh!

lpizzinidev commented Nov 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aws-cdk-automation commented Nov 22, 2023

Uh oh!

aws-cdk-automation commented Nov 23, 2023

Uh oh!

aws-cdk-automation commented Nov 24, 2023

Uh oh!

aws-cdk-automation commented Nov 25, 2023

Uh oh!

aws-cdk-automation commented Nov 26, 2023

Uh oh!

lpizzinidev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lpizzinidev Dec 7, 2023

Choose a reason for hiding this comment

Uh oh!

daschaa Dec 7, 2023

Choose a reason for hiding this comment

Uh oh!

aws-cdk-automation commented Dec 11, 2023

Uh oh!

aws-cdk-automation commented Dec 12, 2023

Uh oh!

aws-cdk-automation commented Dec 13, 2023

Uh oh!

aws-cdk-automation commented Dec 14, 2023

Uh oh!

aws-cdk-automation commented Dec 15, 2023

Uh oh!

aws-cdk-automation commented Dec 16, 2023

Uh oh!

kaizencc left a comment

Choose a reason for hiding this comment

Uh oh!

kaizencc Dec 18, 2023

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Dec 18, 2023

Uh oh!

mergify bot commented Dec 18, 2023

Uh oh!

mergify bot commented Dec 18, 2023

Uh oh!

aws-cdk-automation commented Dec 18, 2023

AWS CodeBuild CI Report

Uh oh!

mergify bot commented Dec 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lpizzinidev left a comment •

edited

Loading

lpizzinidev commented Nov 5, 2023 •

edited

Loading