fix: Iservice for ecsdeployaction #6203

ahammond · 2020-02-10T18:04:59Z

Replaces #5939

This is still WIP. I need help with understanding the fromAttributes implementation. Also, do we want to use the Regex matchers I've proposed, and... if so, what error handling should I be adding when the match fails? Alternatively, I could just do something like

public readonly serviceName = fargateServiceArn.substring(fargateServiceArn.indexOf('/') + 1)

which is simpler, but doesn't provide the same level of validation. Also... is there a standard ARN Regex matcher I should be using instead of rolling my own?

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

aws-cdk-automation · 2020-02-10T18:20:14Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-02-10T18:23:23Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

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

skinny85

This is a good start @ahammond :).

Also: make sure you add unit tests in the @aws-cdk/aws-ecs package for the new methods fromEc2ServiceAttributes and fromFargateServiceAttributes. A unit test for the new capability of the EcsDeployAction also wouldn't be the worst idea :).

packages/@aws-cdk/aws-ecs/lib/base/base-service.ts

packages/@aws-cdk/aws-ecs/lib/ec2/ec2-service.ts

skinny85 · 2020-02-10T18:59:34Z

packages/@aws-cdk/aws-ecs/lib/ec2/ec2-service.ts

+  /**
+   * Import a service definition from the specified service attributes.
+   */
+  public static fromEc2ServiceAttributes(scope: Construct, id: string, attributes: string): IBaseService {


fromEc2ServiceAttributes needs a struct as the last argument, not a string. Something like this:

export interface Ec2ServiceAttributes { /** One of this, or {@link serviceName}, is required. */ public readonly serviceArn?: string; /** One of this, or {@link serviceArn}, is required. */ public readonly serviceName?: string; public readonly cluster: ICluster; } public static fromEc2ServiceAttributes(scope: Construct, id: string, attributes: Ec2ServiceAttributes): IBaseService { // ... }

Take a look at the fromBucketAttributes method in S3 for inspiration.

skinny85 · 2020-02-10T19:01:05Z

packages/@aws-cdk/aws-ecs/lib/fargate/fargate-service.ts

+    return new Import(scope, id);
+  }
+
+
  /**
   * Constructs a new instance of the FargateService class.
   */


Same exact comments in this file as in ec2-service.ts (use Stack.parseArn, new interface FargateServiceAttributes).

packages/@aws-cdk/aws-ecs/lib/base/base-service.ts

skinny85 · 2020-02-10T19:03:14Z

packages/@aws-cdk/aws-ecs/lib/ec2/ec2-service.ts

+    return new Import(scope, id)
+  }
+
+


Unnecessary empty line.

skinny85 · 2020-02-10T19:03:21Z

packages/@aws-cdk/aws-ecs/lib/fargate/fargate-service.ts

+    return new Import(scope, id);
+  }
+
+


Unnecessary empty line.

Pull request has been modified.

aws-cdk-automation · 2020-02-10T21:30:21Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-02-10T21:35:55Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-02-10T21:40:20Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

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

skinny85 · 2020-02-24T16:14:52Z

@ahammond how's it going? Do you need any help with the PR?

skinny85 · 2020-03-12T17:57:34Z

Closing this one now that #6412 is merged.

Bumps [axios](https://github.com/axios/axios) from 0.27.2 to 1.6.8. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/axios/axios/releases">axios's releases</a>.</em></p> <blockquote> <h2>Release v1.6.8</h2> <h2>Release notes:</h2> <h3>Bug Fixes</h3> <ul> <li><strong>AxiosHeaders:</strong> fix AxiosHeaders conversion to an object during config merging (<a href="https://redirect.github.com/axios/axios/issues/6243">#6243</a>) (<a href="https://github.com/axios/axios/commit/2656612bc10fe2757e9832b708ed773ab340b5cb">2656612</a>)</li> <li><strong>import:</strong> use named export for EventEmitter; (<a href="https://github.com/axios/axios/commit/7320430aef2e1ba2b89488a0eaf42681165498b1">7320430</a>)</li> <li><strong>vulnerability:</strong> update follow-redirects to 1.15.6 (<a href="https://redirect.github.com/axios/axios/issues/6300">#6300</a>) (<a href="https://github.com/axios/axios/commit/8786e0ff55a8c68d4ca989801ad26df924042e27">8786e0f</a>)</li> </ul> <h3>Contributors to this release</h3> <ul> <li> <a href="https://github.com/jasonsaayman" title="+4572/-3446 ([#6238](axios/axios#6238) )">Jay</a></li> <li> <a href="https://github.com/DigitalBrainJS" title="+30/-0 ([#6231](axios/axios#6231) )">Dmitriy Mozgovoy</a></li> <li> <a href="https://github.com/Creaous" title="+9/-9 ([#6300](axios/axios#6300) )">Mitchell</a></li> <li> <a href="https://github.com/mannoeu" title="+2/-2 ([#6196](axios/axios#6196) )">Emmanuel</a></li> <li> <a href="https://github.com/ljkeller" title="+3/-0 ([#6194](axios/axios#6194) )">Lucas Keller</a></li> <li> <a href="https://github.com/ADITYA-176" title="+1/-1 ()">Aditya Mogili</a></li> <li> <a href="https://github.com/petrovmiroslav" title="+1/-1 ([#6243](axios/axios#6243) )">Miroslav Petrov</a></li> </ul> <h2>Release v1.6.7</h2> <h2>Release notes:</h2> <h3>Bug Fixes</h3> <ul> <li>capture async stack only for rejections with native error objects; (<a href="https://redirect.github.com/axios/axios/issues/6203">#6203</a>) (<a href="https://github.com/axios/axios/commit/1a08f90f402336e4d00e9ee82f211c6adb1640b0">1a08f90</a>)</li> </ul> <h3>Contributors to this release</h3> <ul> <li> <a href="https://github.com/DigitalBrainJS" title="+30/-26 ([#6203](axios/axios#6203) )">Dmitriy Mozgovoy</a></li> <li> <a href="https://github.com/zh-lx" title="+0/-3 ([#6186](axios/axios#6186) )">zhoulixiang</a></li> </ul> <h2>Release v1.6.6</h2> <h2>Release notes:</h2> <h3>Bug Fixes</h3> <ul> <li>fixed missed dispatchBeforeRedirect argument (<a href="https://redirect.github.com/axios/axios/issues/5778">#5778</a>) (<a href="https://github.com/axios/axios/commit/a1938ff073fcb0f89011f001dfbc1fa1dc995e39">a1938ff</a>)</li> <li>wrap errors to improve async stack trace (<a href="https://redirect.github.com/axios/axios/issues/5987">#5987</a>) (<a href="https://github.com/axios/axios/commit/123f354b920f154a209ea99f76b7b2ef3d9ebbab">123f354</a>)</li> </ul> <h3>Contributors to this release</h3> <ul> <li> <a href="https://github.com/ikonst" title="+91/-8 ([#5987](axios/axios#5987) )">Ilya Priven</a></li> <li> <a href="https://github.com/zaosoula" title="+6/-6 ([#5778](axios/axios#5778) )">Zao Soula</a></li> </ul> <h2>Release v1.6.5</h2> <h2>Release notes:</h2> <h3>Bug Fixes</h3> <ul> <li><strong>ci:</strong> refactor notify action as a job of publish action; (<a href="https://redirect.github.com/axios/axios/issues/6176">#6176</a>) (<a href="https://github.com/axios/axios/commit/0736f95ce8776366dc9ca569f49ba505feb6373c">0736f95</a>)</li> <li><strong>dns:</strong> fixed lookup error handling; (<a href="https://redirect.github.com/axios/axios/issues/6175">#6175</a>) (<a href="https://github.com/axios/axios/commit/f4f2b039dd38eb4829e8583caede4ed6d2dd59be">f4f2b03</a>)</li> </ul> <h3>Contributors to this release</h3> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/axios/axios/blob/v1.x/CHANGELOG.md">axios's changelog</a>.</em></p> <blockquote> <h2><a href="https://github.com/axios/axios/compare/v1.6.7...v1.6.8">1.6.8</a> (2024-03-15)</h2> <h3>Bug Fixes</h3> <ul> <li><strong>AxiosHeaders:</strong> fix AxiosHeaders conversion to an object during config merging (<a href="https://redirect.github.com/axios/axios/issues/6243">#6243</a>) (<a href="https://github.com/axios/axios/commit/2656612bc10fe2757e9832b708ed773ab340b5cb">2656612</a>)</li> <li><strong>import:</strong> use named export for EventEmitter; (<a href="https://github.com/axios/axios/commit/7320430aef2e1ba2b89488a0eaf42681165498b1">7320430</a>)</li> <li><strong>vulnerability:</strong> update follow-redirects to 1.15.6 (<a href="https://redirect.github.com/axios/axios/issues/6300">#6300</a>) (<a href="https://github.com/axios/axios/commit/8786e0ff55a8c68d4ca989801ad26df924042e27">8786e0f</a>)</li> </ul> <h3>Contributors to this release</h3> <ul> <li> <a href="https://github.com/jasonsaayman" title="+4572/-3446 ([#6238](axios/axios#6238) )">Jay</a></li> <li> <a href="https://github.com/DigitalBrainJS" title="+30/-0 ([#6231](axios/axios#6231) )">Dmitriy Mozgovoy</a></li> <li> <a href="https://github.com/Creaous" title="+9/-9 ([#6300](axios/axios#6300) )">Mitchell</a></li> <li> <a href="https://github.com/mannoeu" title="+2/-2 ([#6196](axios/axios#6196) )">Emmanuel</a></li> <li> <a href="https://github.com/ljkeller" title="+3/-0 ([#6194](axios/axios#6194) )">Lucas Keller</a></li> <li> <a href="https://github.com/ADITYA-176" title="+1/-1 ()">Aditya Mogili</a></li> <li> <a href="https://github.com/petrovmiroslav" title="+1/-1 ([#6243](axios/axios#6243) )">Miroslav Petrov</a></li> </ul> <h2><a href="https://github.com/axios/axios/compare/v1.6.6...v1.6.7">1.6.7</a> (2024-01-25)</h2> <h3>Bug Fixes</h3> <ul> <li>capture async stack only for rejections with native error objects; (<a href="https://redirect.github.com/axios/axios/issues/6203">#6203</a>) (<a href="https://github.com/axios/axios/commit/1a08f90f402336e4d00e9ee82f211c6adb1640b0">1a08f90</a>)</li> </ul> <h3>Contributors to this release</h3> <ul> <li> <a href="https://github.com/DigitalBrainJS" title="+30/-26 ([#6203](axios/axios#6203) )">Dmitriy Mozgovoy</a></li> <li> <a href="https://github.com/zh-lx" title="+0/-3 ([#6186](axios/axios#6186) )">zhoulixiang</a></li> </ul> <h2><a href="https://github.com/axios/axios/compare/v1.6.5...v1.6.6">1.6.6</a> (2024-01-24)</h2> <h3>Bug Fixes</h3> <ul> <li>fixed missed dispatchBeforeRedirect argument (<a href="https://redirect.github.com/axios/axios/issues/5778">#5778</a>) (<a href="https://github.com/axios/axios/commit/a1938ff073fcb0f89011f001dfbc1fa1dc995e39">a1938ff</a>)</li> <li>wrap errors to improve async stack trace (<a href="https://redirect.github.com/axios/axios/issues/5987">#5987</a>) (<a href="https://github.com/axios/axios/commit/123f354b920f154a209ea99f76b7b2ef3d9ebbab">123f354</a>)</li> </ul> <h3>Contributors to this release</h3> <ul> <li> <a href="https://github.com/ikonst" title="+91/-8 ([#5987](axios/axios#5987) )">Ilya Priven</a></li> <li> <a href="https://github.com/zaosoula" title="+6/-6 ([#5778](axios/axios#5778) )">Zao Soula</a></li> </ul> <h2><a href="https://github.com/axios/axios/compare/v1.6.4...v1.6.5">1.6.5</a> (2024-01-05)</h2> <h3>Bug Fixes</h3> <ul> <li><strong>ci:</strong> refactor notify action as a job of publish action; (<a href="https://redirect.github.com/axios/axios/issues/6176">#6176</a>) (<a href="https://github.com/axios/axios/commit/0736f95ce8776366dc9ca569f49ba505feb6373c">0736f95</a>)</li> </ul> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/axios/axios/commit/ab3f0f9a94853c821cb00f1112788ecdd3ae7ed1"><code>ab3f0f9</code></a> chore(release): v1.6.8 (<a href="https://redirect.github.com/axios/axios/issues/6303">#6303</a>)</li> <li><a href="https://github.com/axios/axios/commit/2656612bc10fe2757e9832b708ed773ab340b5cb"><code>2656612</code></a> fix(AxiosHeaders): fix AxiosHeaders conversion to an object during config mer...</li> <li><a href="https://github.com/axios/axios/commit/7320430aef2e1ba2b89488a0eaf42681165498b1"><code>7320430</code></a> fix(import): use named export for EventEmitter;</li> <li><a href="https://github.com/axios/axios/commit/8786e0ff55a8c68d4ca989801ad26df924042e27"><code>8786e0f</code></a> fix(vulnerability): update follow-redirects to 1.15.6 (<a href="https://redirect.github.com/axios/axios/issues/6300">#6300</a>)</li> <li><a href="https://github.com/axios/axios/commit/d844227411263fab39d447442879112f8b0c8de5"><code>d844227</code></a> chore: update and bump deps (<a href="https://redirect.github.com/axios/axios/issues/6238">#6238</a>)</li> <li><a href="https://github.com/axios/axios/commit/caa06252015003990958d7db96f63aa646bc58e8"><code>caa0625</code></a> docs: update README responseEncoding types (<a href="https://redirect.github.com/axios/axios/issues/6194">#6194</a>)</li> <li><a href="https://github.com/axios/axios/commit/41c4584a41fad1d94cf86331667deff5a0b75044"><code>41c4584</code></a> docs: Update README.md to point to current axios version in CDN links (<a href="https://redirect.github.com/axios/axios/issues/6196">#6196</a>)</li> <li><a href="https://github.com/axios/axios/commit/bf6974f16af6c72985f094a98d874c5a6adcdc83"><code>bf6974f</code></a> chore(ci): add npm tag action; (<a href="https://redirect.github.com/axios/axios/issues/6231">#6231</a>)</li> <li><a href="https://github.com/axios/axios/commit/a52e4d9af51205959ef924f87bcf90c605e08a1e"><code>a52e4d9</code></a> chore(release): v1.6.7 (<a href="https://redirect.github.com/axios/axios/issues/6204">#6204</a>)</li> <li><a href="https://github.com/axios/axios/commit/2b69888dd5601bbc872452ccd24010219fb6e41a"><code>2b69888</code></a> chore: remove unnecessary check (<a href="https://redirect.github.com/axios/axios/issues/6186">#6186</a>)</li> <li>Additional commits viewable in <a href="https://github.com/axios/axios/compare/v0.27.2...v1.6.8">compare view</a></li> </ul> </details> <br /> [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=axios&package-manager=npm_and_yarn&previous-version=0.27.2&new-version=1.6.8)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/aws/aws-cdk/network/alerts). </details>

ahammond requested a review from skinny85 February 10, 2020 18:05

ahammond force-pushed the iservice_for_ecsdeployaction branch from ea2247e to 1198e88 Compare February 10, 2020 18:08

skinny85 suggested changes Feb 10, 2020

View reviewed changes

skinny85 previously requested changes Feb 10, 2020

View reviewed changes

SomayaB assigned skinny85 Feb 10, 2020

ahammond force-pushed the iservice_for_ecsdeployaction branch from 1198e88 to 1b9a8ea Compare February 10, 2020 21:15

ahammond force-pushed the iservice_for_ecsdeployaction branch from 1b9a8ea to 8ac8a6c Compare February 10, 2020 21:20

Andrew Hammond added 2 commits February 10, 2020 13:24

fix: include serviceName in IService

4f8036f

WIP: cluster support

f176cde

ahammond force-pushed the iservice_for_ecsdeployaction branch from 8ac8a6c to f176cde Compare February 10, 2020 21:24

skinny85 mentioned this pull request Feb 12, 2020

CodePipeline EcsDeployAction - unable to refer existing Fargate Service #6178

Closed

skinny85 mentioned this pull request Feb 24, 2020

fix(codepipeline-actions): use IBaseService instead of BaseService in EcsDeployActionProps #6412

Merged

skinny85 closed this Mar 12, 2020

ahammond deleted the iservice_for_ecsdeployaction branch March 12, 2020 20:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Iservice for ecsdeployaction #6203

fix: Iservice for ecsdeployaction #6203

ahammond commented Feb 10, 2020

aws-cdk-automation commented Feb 10, 2020

aws-cdk-automation commented Feb 10, 2020

skinny85 left a comment •

edited

Loading

skinny85 Feb 10, 2020

skinny85 Feb 10, 2020

skinny85 Feb 10, 2020

skinny85 Feb 10, 2020

aws-cdk-automation commented Feb 10, 2020

aws-cdk-automation commented Feb 10, 2020

aws-cdk-automation commented Feb 10, 2020

skinny85 commented Feb 24, 2020

skinny85 commented Mar 12, 2020

fix: Iservice for ecsdeployaction #6203

fix: Iservice for ecsdeployaction #6203

Conversation

ahammond commented Feb 10, 2020

aws-cdk-automation commented Feb 10, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Feb 10, 2020

AWS CodeBuild CI Report

skinny85 left a comment • edited Loading

Choose a reason for hiding this comment

skinny85 Feb 10, 2020

Choose a reason for hiding this comment

skinny85 Feb 10, 2020

Choose a reason for hiding this comment

skinny85 Feb 10, 2020

Choose a reason for hiding this comment

skinny85 Feb 10, 2020

Choose a reason for hiding this comment

aws-cdk-automation commented Feb 10, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Feb 10, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Feb 10, 2020

AWS CodeBuild CI Report

skinny85 commented Feb 24, 2020

skinny85 commented Mar 12, 2020

skinny85 left a comment •

edited

Loading