refactor(cli): bootstrap & deployments are using IoHost #33451

mrgrain · 2025-02-14T15:31:25Z

Reason for this change

Make bootstrap and deployment api code use the new IoHost interface.
This will be required so we can accurately track the action for each log.

Description of changes

Change a lot of logging to use the IoHost.

There is a temporary annoyance that we also have to pass the action. In future we will likely replace this with an internal IoHost version that is action aware. To make this easier for now, a new temporary interface IoMessaging is introduced. This is also why I'm using restructuring { ioHost, action } in many places. To idea being that removing the action later on will be simpler or at least give cleaner diff.

Describe any new or updated permissions being added

n/a

Description of how you validated changes

existing tests

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

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 fails with the following errors:

❌ CodeCov is indicating a drop in code coverage

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.

kaizencc

at its core the payout of this PR should be a 1-1 mapping of the existing logging functions in bootstrap & deployments to ioHost.notify() right?

are there any particular parts of this PR that warrant additional scrutiny? i might miss it given the large diff of passing { ioHost, action } everywhere.

kaizencc · 2025-02-14T21:02:12Z

packages/@aws-cdk/toolkit/lib/toolkit/toolkit.ts

    return new Deployments({
      sdkProvider: await this.sdkProvider(action),
      toolkitStackName: this.toolkitStackName,
+      ioHost: this.ioHost as any, // @todo temporary while we have to separate IIoHost interfaces


why do we have separate IIoHost interfaces?

Because currently the toolkit depends on the CLI package (and therefore the CLI cannot depend on the toolkit). Once the repo split is complete, we will introduce a new private intermediate package that both toolkit and CLI can depend on.

packages/aws-cdk/lib/api/deployments/assets.ts

kaizencc · 2025-02-14T21:09:36Z

packages/aws-cdk/lib/api/deployments/cloudformation.ts

 }

-export async function cleanupOldChangeset(changeSetName: string, stackName: string, cfn: ICloudFormationClient) {
+async function cleanupOldChangeset(


does cleanupOldChangeset not get exposed to users of aws-cdk? asking if you've checked that it's safe to unexport this function

It's only use is inside this file. We have previously compiled a list of symbols that are "externally used" and this is not part of it.

mrgrain · 2025-02-14T21:17:57Z

at its core the payout of this PR should be a 1-1 mapping of the existing logging functions in bootstrap & deployments to ioHost.notify() right?

are there any particular parts of this PR that warrant additional scrutiny? i might miss it given the large diff of passing { ioHost, action } everywhere.

That's correct. asset-publishing.ts and deployments.ts at the bottom have a logger class changing.

mrgrain · 2025-02-14T21:18:20Z

packages/aws-cdk/lib/api/deployments/asset-publishing.ts

@kaizencc special attention

mrgrain · 2025-02-14T21:18:38Z

packages/aws-cdk/lib/api/deployments/deployments.ts

+class ParallelSafeAssetProgress extends BasePublishProgressListener {
+  private readonly prefix: string;
+
+  constructor(prefix: string, { ioHost, action }: IoMessaging) {
+    super({ ioHost, action });
+    this.prefix = prefix;
+  }
+
+  protected getMessage(type: cdk_assets.EventType, event: cdk_assets.IPublishProgress): string {
+    return `${this.prefix}${type}: ${event.message}`;


@kaizencc special attention

codecov · 2025-02-14T21:34:45Z

Codecov Report

Attention: Patch coverage is 81.25000% with 30 lines in your changes missing coverage. Please review.

Project coverage is 81.01%. Comparing base (523e0f0) to head (4f74068).
Report is 13 commits behind head on main.

❌ Your patch status has failed because the patch coverage (81.25%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #33451      +/-   ##
==========================================
+ Coverage   81.00%   81.01%   +0.01%     
==========================================
  Files         238      238              
  Lines       14271    14290      +19     
  Branches     2492     2494       +2     
==========================================
+ Hits        11560    11577      +17     
- Misses       2425     2426       +1     
- Partials      286      287       +1

Flag	Coverage Δ
suite.unit	`81.01% <81.25%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	`79.95% <81.25%> (+0.02%)`	⬆️
packages/aws-cdk-lib/core	`82.16% <ø> (ø)`

aws-cdk-automation · 2025-02-14T21:52:55Z

AWS CodeBuild CI Report

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

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

aws-cdk-automation · 2025-02-14T22:45:37Z

➡️ PR build request submitted to test-main-pipeline ⬅️

A maintainer must now check the pipeline and add the pr-linter/cli-integ-tested label once the pipeline succeeds.

mrgrain · 2025-02-17T19:34:18Z

Closed in favor of aws/aws-cdk-cli#40

github-actions · 2025-02-17T19:34:22Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

mrgrain added 3 commits February 14, 2025 15:29

refactor(cli): deployments are using IoHost

29838be

asset-publishing

d68a6d4

deployments & bootstrap

9978b74

mrgrain requested a review from a team as a code owner February 14, 2025 15:31

aws-cdk-automation requested a review from a team February 14, 2025 15:31

github-actions bot added the p2 label Feb 14, 2025

mergify bot added the contribution/core This is a PR that came from AWS. label Feb 14, 2025

mrgrain had a problem deploying to test-pipeline February 14, 2025 15:32 — with GitHub Actions Failure

aws-cdk-automation requested changes Feb 14, 2025

View reviewed changes

bootstrap

b1a696c

mrgrain had a problem deploying to test-pipeline February 14, 2025 16:33 — with GitHub Actions Failure

mrgrain had a problem deploying to test-pipeline February 14, 2025 16:51 — with GitHub Actions Failure

deployments pt2

95301d2

mrgrain force-pushed the mrgrain/chore/cli/deployments-io-host branch from 59eea4d to 3668ceb Compare February 14, 2025 19:55

mrgrain had a problem deploying to test-pipeline February 14, 2025 19:56 — with GitHub Actions Failure

everything

f8a379b

mrgrain force-pushed the mrgrain/chore/cli/deployments-io-host branch from 3668ceb to f8a379b Compare February 14, 2025 20:41

mrgrain had a problem deploying to test-pipeline February 14, 2025 20:42 — with GitHub Actions Failure

kaizencc reviewed Feb 14, 2025

View reviewed changes

mrgrain commented Feb 14, 2025

View reviewed changes

packages/aws-cdk/lib/api/deployments/asset-publishing.ts

Copy link

Contributor Author

mrgrain Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaizencc special attention

mrgrain commented Feb 14, 2025

View reviewed changes

fixup

4f74068

mrgrain temporarily deployed to test-pipeline February 14, 2025 21:23 — with GitHub Actions Inactive

mrgrain added pr/do-not-merge This PR should not be merged at this time. pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested labels Feb 17, 2025

mrgrain closed this Feb 17, 2025

github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2025

mrgrain deleted the mrgrain/chore/cli/deployments-io-host branch May 12, 2025 16:29

refactor(cli): bootstrap & deployments are using IoHost #33451

refactor(cli): bootstrap & deployments are using IoHost #33451

Uh oh!

Conversation

mrgrain commented Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reason for this change

Description of changes

Describe any new or updated permissions being added

Description of how you validated changes

Checklist

Uh oh!

aws-cdk-automation 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!

kaizencc left a comment

Choose a reason for hiding this comment

Uh oh!

kaizencc Feb 14, 2025

Choose a reason for hiding this comment

Uh oh!

mrgrain Feb 14, 2025 • 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!

kaizencc Feb 14, 2025

Choose a reason for hiding this comment

Uh oh!

mrgrain Feb 14, 2025

Choose a reason for hiding this comment

Uh oh!

mrgrain commented Feb 14, 2025

Uh oh!

mrgrain Feb 14, 2025

Choose a reason for hiding this comment

Uh oh!

mrgrain Feb 14, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

aws-cdk-automation commented Feb 14, 2025

AWS CodeBuild CI Report

Uh oh!

aws-cdk-automation commented Feb 14, 2025

Uh oh!

mrgrain commented Feb 17, 2025

Uh oh!

github-actions bot commented Feb 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mrgrain commented Feb 14, 2025 •

edited

Loading

aws-cdk-automation left a comment •

edited

Loading

mrgrain Feb 14, 2025 •

edited

Loading

codecov bot commented Feb 14, 2025 •

edited

Loading