feat(toolkit-lib): additional messages contain structured data #193
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #193 +/- ##
==========================================
+ Coverage 85.02% 85.22% +0.19%
==========================================
Files 207 207
Lines 35706 35706
Branches 4636 4636
==========================================
+ Hits 30360 30431 +71
+ Misses 5188 5123 -65
+ Partials 158 152 -6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| export const warn = <T = never>(details: CodeInfoMaybeInterface<T>) => generic<T>('warn', details); | ||
| export const error = <T = never>(details: CodeInfoMaybeInterface<T>) => generic<T>('error', details); | ||
| export const result = <T extends object>(details: Required<CodeInfo>) => generic<T extends object ? T : never>('result', details); | ||
| export const result = <T extends object = ImpossibleType>(details: Required<CodeInfo>) => generic<T extends object ? T : never>('result', details); |
There was a problem hiding this comment.
With this, attempting to post a result message without data will now fail at compile time. Unfortunately it's not possible in TS to mark a generic type as required. Instead we are defaulting to a type that is impossible for a user to create, and thus their code will always fail unless a different generic is provided.
mrgrain
force-pushed
the
mrgrain/feat/emit-more-structured-data
branch
5 times, most recently
from
3f26818 to
1e6fb5e
Compare
There was a problem hiding this comment.
these were just moved into a separate file
There was a problem hiding this comment.
split into 2 separate files
There was a problem hiding this comment.
removed in favor of using CODES.CDK_ASSEMBLY_I0150.msg()
There was a problem hiding this comment.
split out from other file
| this._sdkProvider = await SdkProvider.withAwsCliCompatibleDefaults({ | ||
| ...this.props.sdkConfig, | ||
| logger: asSdkLogger(this.ioHost, action), | ||
| logger: asSdkLogger(withAction(this.ioHost, action)), |
There was a problem hiding this comment.
small refactor so the private SdkLogger can just take an ActionAwareIoHost. This makes life much easier inside the SdkLogger
| DEFAULT_TOOLKIT_INFO: make.info({ | ||
| code: 'CDK_TOOLKIT_I0000', | ||
| description: 'Default info messages emitted from the Toolkit', | ||
| }), | ||
| DEFAULT_TOOLKIT_DEBUG: make.debug({ | ||
| code: 'CDK_TOOLKIT_I0000', | ||
| description: 'Default debug messages emitted from the Toolkit', | ||
| }), | ||
| DEFAULT_TOOLKIT_WARN: make.warn({ | ||
| code: 'CDK_TOOLKIT_W0000', | ||
| description: 'Default warning messages emitted from the Toolkit', | ||
| }), |
There was a problem hiding this comment.
These are some generic messages without data that don't deserve a special code. Usually because there already is a message with data preceding it.
| const cloudWatchLogMonitor = options.traceLogs ? new CloudWatchLogEventMonitor() : undefined; | ||
| const deployAndWatch = async () => { | ||
| latch = 'deploying'; | ||
| latch = 'deploying' as LatchState; |
There was a problem hiding this comment.
Makes L636 a bit more readable.
mrgrain
force-pushed
the
mrgrain/feat/emit-more-structured-data
branch
from
1e6fb5e to
f2a26c8
Compare
There was a problem hiding this comment.
removed in favor of using CODES.CDK_ASSEMBLY_I0150.msg()
There was a problem hiding this comment.
Test changed on purpose. I upgraded this particular message from info to result as it's the main outcome of the destroy action. This is fine because the message was previously uncoded (i.e. using a default code) and we explicitly allow that.
packages/@aws-cdk/tmp-toolkit-helpers/src/api/io/private/message-maker.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/tmp-toolkit-helpers/src/api/io/private/message-maker.ts
Outdated
Show resolved
Hide resolved
| CDK_TOOLKIT_I7900: make.result<cxapi.CloudFormationStackArtifact>({ | ||
| code: 'CDK_TOOLKIT_I7900', | ||
| description: 'Stack deletion succeeded', | ||
| interface: 'cxapi.CloudFormationStackArtifact', |
There was a problem hiding this comment.
already discussed but note that this will result in a bad link in the CODE_REGISTRY
There was a problem hiding this comment.
Acceptable right now. Will fix later.
| const assembly = await assemblyFromSource(cx, false); | ||
| const ioHost = withAction(this.ioHost, 'watch'); | ||
| const rootDir = options.watchDir ?? process.cwd(); | ||
| await ioHost.notify(debug(`root directory used for 'watch' is: ${rootDir}`)); |
There was a problem hiding this comment.
why remove these comments from watch?
There was a problem hiding this comment.
They are moved further down to be emitted as a single message.
Co-authored-by: Eli Polonsky <[email protected]>
Signed-off-by: github-actions <[email protected]>
4 participants
Adds many more explicit codes and message payload to messages that previously were default messages.
Within
toolkit-libreplace all previous message helpers with the new Message Maker pattern:CODES.CDK_CAT_I1234.msg("my message").By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license