refactor(cli): stack monitor uses io-host #150

mrgrain · 2025-02-25T16:09:05Z

Provides stack events to the IoHost in a structured way.

The main change here is that StackActivityMonitor does not directly talk to the ActivityPrinter anymore. Instead messages are emitted to the IoHost. This caused two related changes:

Progress tracking has been split into a more generic StackProgressMonitor and progress information is now provided directly by the StackActivityMonitor
Originally we only had a single timer that covered polling events and printing them. With the split, we now have two separate timers: one for polling events and a separate one for updating the view

Removed low-level options:

progress -> now a CliIoHost property
ci -> use value from CliIoHost directly instead of passing through all the layers
quiet -> surprisingly this was unused by CLI. We should have something like this again, but can be implemented in a follow up PR.

Manually tested the changes in addition to integ tests.

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

codecov-commenter · 2025-02-26T14:50:54Z

Codecov Report

Attention: Patch coverage is 73.03719% with 261 lines in your changes missing coverage. Please review.

Project coverage is 84.46%. Comparing base (97d02a8) to head (79285c2).

Files with missing lines	Patch %	Lines
...ckages/aws-cdk/lib/cli/activity-printer/current.ts	27.43%	119 Missing ⚠️
...ckages/aws-cdk/lib/cli/activity-printer/history.ts	66.20%	47 Missing and 2 partials ⚠️
packages/aws-cdk/lib/toolkit/cli-io-host.ts	74.19%	24 Missing ⚠️
...cdk/lib/api/stack-events/stack-activity-monitor.ts	87.22%	22 Missing and 1 partial ⚠️
packages/aws-cdk/lib/cli/activity-printer/base.ts	86.36%	21 Missing ⚠️
...ckages/aws-cdk/lib/api/deployments/deploy-stack.ts	59.09%	9 Missing ⚠️
packages/aws-cdk/lib/cli/cdk-toolkit.ts	11.11%	8 Missing ⚠️
...cdk/lib/api/stack-events/stack-progress-monitor.ts	95.34%	6 Missing ⚠️
...ackages/aws-cdk/lib/api/deployments/deployments.ts	91.66%	1 Missing ⚠️
packages/aws-cdk/lib/util/cloudformation.ts	96.15%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #150      +/-   ##
==========================================
- Coverage   84.79%   84.46%   -0.34%     
==========================================
  Files         198      204       +6     
  Lines       35400    35646     +246     
  Branches     4573     4563      -10     
==========================================
+ Hits        30019    30108      +89     
- Misses       5235     5393     +158     
+ Partials      146      145       -1

Flag	Coverage Δ
suite.unit	`84.46% <73.03%> (-0.34%)`	⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mrgrain · 2025-02-26T15:23:46Z

packages/aws-cdk/test/api/stack-events/stack-monitor.test.ts

renamed to stack-activity-monitor.test.ts

mrgrain · 2025-02-26T15:26:00Z

packages/aws-cdk/lib/util/string-manipulation.ts

We had multiple versions of this. Consolidating to one.

mrgrain · 2025-02-26T15:27:09Z

packages/aws-cdk/lib/util/cloudformation.ts

Pulling these out into a shared file.

mrgrain · 2025-02-26T15:27:33Z

packages/aws-cdk/lib/commands/deploy.ts

This is now only a CLI concern (not toolkit anymore)

mrgrain · 2025-02-27T11:43:47Z

packages/aws-cdk/lib/api/stack-events/stack-activity-monitor.ts

-function padRight(n: number, x: string): string {
-  return x + ' '.repeat(Math.max(0, n - x.length));
-}
-
-/**
- * Infamous padLeft()
- */
-function padLeft(n: number, x: string): string {
-  return ' '.repeat(Math.max(0, n - x.length)) + x;
-}


moved to util/string-manipulation.ts

mrgrain · 2025-02-27T11:44:00Z

packages/aws-cdk/lib/api/stack-events/stack-activity-monitor.ts

-function calcMaxResourceTypeLength(template: any) {
-  const resources = (template && template.Resources) || {};
-  let maxWidth = 0;
-  for (const id of Object.keys(resources)) {
-    const type = resources[id].Type || '';
-    if (type.length > maxWidth) {
-      maxWidth = type.length;
-    }
-  }
-  return maxWidth;
-}


moved to util/cloudformation.ts

mrgrain · 2025-02-27T11:45:03Z

packages/aws-cdk/lib/api/stack-events/stack-activity-monitor.ts

-  return maxWidth;
-}
-
-interface PrinterProps {


Rest of this file moved to various files in cli/activity-printer but also got refactored a lot

packages/@aws-cdk/toolkit-lib/lib/api/io/private/codes.ts

rix0rrr · 2025-02-27T12:14:36Z

packages/@aws-cdk/toolkit-lib/lib/api/io/private/codes.ts

+    code: 'CDK_TOOLKIT_I5502',
+    description: 'Stack Monitoring Activity event',
+    level: 'info',
+    interface: 'StackActivity',


Do we have our own copy of this type? I'm not comfortable exposing other people's types as part of our API.

Also if we make our own copy of the type, we'll have the ability to add fields if we feel that's convenient, and we can simplify and omit fields that aren't necessary for our current implementation.

This is our own copy of StackActivity.

However its event property is typed as @aws-sdk/client-cloudformation.StackEvent. I'm inclined to pass this on as is, since the CFN API is well defined und unlikely to change badly or even at all.

packages/aws-cdk/lib/cli/activity-printer/base.ts

packages/aws-cdk/lib/cli/activity-printer/current.ts

packages/aws-cdk/lib/api/stack-events/stack-activity-monitor.ts

mrgrain requested a deployment to integ-approval February 25, 2025 16:09 — with GitHub Actions Waiting

mrgrain force-pushed the mrgrain/refactor/io-host-stack-monitor branch from f4fecad to e8b93d8 Compare February 26, 2025 14:43

mrgrain requested a deployment to integ-approval February 26, 2025 14:43 — with GitHub Actions Waiting

mrgrain force-pushed the mrgrain/refactor/io-host-stack-monitor branch from e8b93d8 to 97a7ca5 Compare February 26, 2025 15:02

mrgrain requested a deployment to integ-approval February 26, 2025 15:02 — with GitHub Actions Waiting

mrgrain requested a deployment to integ-approval February 26, 2025 15:22 — with GitHub Actions Waiting

mrgrain commented Feb 26, 2025

View reviewed changes

packages/aws-cdk/test/api/stack-events/stack-monitor.test.ts Outdated

Copy link

Contributor Author

mrgrain Feb 26, 2025

Choose a reason for hiding this comment

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

renamed to stack-activity-monitor.test.ts

mrgrain commented Feb 26, 2025

View reviewed changes

packages/aws-cdk/lib/util/string-manipulation.ts Outdated

Copy link

Contributor Author

mrgrain Feb 26, 2025

Choose a reason for hiding this comment

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

We had multiple versions of this. Consolidating to one.

mrgrain commented Feb 26, 2025

View reviewed changes

packages/aws-cdk/lib/util/cloudformation.ts Outdated

Copy link

Contributor Author

mrgrain Feb 26, 2025

Choose a reason for hiding this comment

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

Pulling these out into a shared file.

mrgrain commented Feb 26, 2025

View reviewed changes

packages/aws-cdk/lib/commands/deploy.ts Outdated

Copy link

Contributor Author

mrgrain Feb 26, 2025

Choose a reason for hiding this comment

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

This is now only a CLI concern (not toolkit anymore)

mrgrain temporarily deployed to integ-approval February 26, 2025 18:54 — with GitHub Actions Inactive

mrgrain force-pushed the mrgrain/refactor/io-host-stack-monitor branch from aaf99a7 to a0fcbbd Compare February 27, 2025 11:24

mrgrain requested a deployment to integ-approval February 27, 2025 11:24 — with GitHub Actions Waiting

mrgrain commented Feb 27, 2025

View reviewed changes

mrgrain marked this pull request as ready for review February 27, 2025 11:47

aws-cdk-automation enabled auto-merge February 27, 2025 11:47

mrgrain disabled auto-merge February 27, 2025 11:48

mrgrain force-pushed the mrgrain/refactor/io-host-stack-monitor branch from a0fcbbd to 1b28383 Compare February 27, 2025 12:34

mrgrain requested a deployment to integ-approval February 27, 2025 12:34 — with GitHub Actions Waiting

rix0rrr reviewed Feb 27, 2025

View reviewed changes

mrgrain force-pushed the mrgrain/refactor/io-host-stack-monitor branch from 1b28383 to 56a8ee3 Compare February 27, 2025 13:47

mrgrain requested a deployment to integ-approval February 27, 2025 13:47 — with GitHub Actions Waiting

aws-cdk-automation requested a deployment to integ-approval February 27, 2025 13:59 — with GitHub Actions Waiting

mrgrain force-pushed the mrgrain/refactor/io-host-stack-monitor branch from 94f39a2 to 1a42d87 Compare February 27, 2025 14:31

mrgrain requested a deployment to integ-approval February 27, 2025 14:31 — with GitHub Actions Waiting

rix0rrr approved these changes Feb 27, 2025

View reviewed changes

refactor(cli): stack monitor uses io-host

79285c2

mrgrain force-pushed the mrgrain/refactor/io-host-stack-monitor branch from 1a42d87 to 79285c2 Compare February 27, 2025 15:54

mrgrain temporarily deployed to integ-approval February 27, 2025 15:54 — with GitHub Actions Inactive

mrgrain enabled auto-merge February 27, 2025 15:54

mrgrain temporarily deployed to run-tests February 27, 2025 16:09 — with GitHub Actions Inactive

mrgrain added this pull request to the merge queue Feb 27, 2025

Merged via the queue into main with commit 0d9912f Feb 27, 2025
20 checks passed

mrgrain deleted the mrgrain/refactor/io-host-stack-monitor branch February 27, 2025 17:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(cli): stack monitor uses io-host #150

refactor(cli): stack monitor uses io-host #150

mrgrain commented Feb 25, 2025 •

edited

Loading

codecov-commenter commented Feb 26, 2025 •

edited

Loading

mrgrain Feb 26, 2025

mrgrain Feb 26, 2025

mrgrain Feb 26, 2025

mrgrain Feb 26, 2025

mrgrain Feb 27, 2025

mrgrain Feb 27, 2025

mrgrain Feb 27, 2025

rix0rrr Feb 27, 2025

mrgrain Feb 27, 2025 •

edited

Loading

refactor(cli): stack monitor uses io-host #150

refactor(cli): stack monitor uses io-host #150

Conversation

mrgrain commented Feb 25, 2025 • edited Loading

codecov-commenter commented Feb 26, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrgrain Feb 27, 2025 • edited Loading

Choose a reason for hiding this comment

mrgrain commented Feb 25, 2025 •

edited

Loading

codecov-commenter commented Feb 26, 2025 •

edited

Loading

mrgrain Feb 27, 2025 •

edited

Loading