fix: route metadata through toolkit-lib - #1747
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
mrgrain
changed the title
mrgrain
changed the title
31a472d to
146d937
Compare
| this.ioHost.once( | ||
| IO.CDK_TOOLKIT_I2901, | ||
| (msg) => ({ | ||
| action: 'metadata', | ||
| message: serializeStructure(msg.data.stacks[0]?.metadata ?? {}, json), | ||
| }), | ||
| ); | ||
| this.ioHost.once( | ||
| IO.CDK_TOOLKIT_I1001, | ||
| () => ({ preventDefault: true }), | ||
| ); | ||
| this.ioHost.once( | ||
| IO.CDK_TOOLKIT_I1000, | ||
| () => ({ preventDefault: true }), | ||
| ); | ||
|
|
||
| try { | ||
| await this.toolkit.list(this.props.cloudExecutable, { | ||
| stacks: { | ||
| patterns: [stackName], | ||
| strategy: StackSelectionStrategy.PATTERN_MUST_MATCH_SINGLE, | ||
| expand: ExpandStackSelection.NONE, | ||
| }, | ||
| }); | ||
| } finally { | ||
| this.ioHost.removeAllListeners(); |
There was a problem hiding this comment.
This is for error-contingent cleanup of the listeners we create with .once right? Why not use the function that removes the listener for each one? Like:
let disposers = [
this.ioHost.once(...),
this.ioHost.once(...),
this.ioHost.once(...)
];
...
try {
...
} finally {
disposers.forEach(cleanupFunc => cleanupFunc());
}There was a problem hiding this comment.
Could do, but this.ioHost.removeAllListeners(); does the same and is already used elsewhere. I think in general we need a better (more generic) way of handling these. Can I do this as a followup?
There was a problem hiding this comment.
It does the same thing now, but would wipe other listeners too, if we introduce them in the future, and might result in an unexpected/difficult to debug situation. But yep, OK as a follow-up.
There was a problem hiding this comment.
Yeah, I see. But then the CLI only ever runs one command... I think about it.
3 participants
The
cdk metadatacommand now delegates synthesis and single-stack selection to the existing toolkit-libToolkit.list()action instead of maintaining a separate legacy selection path. This keeps the CLI thin while preserving the command's YAML and--jsonoutput.CliIoHostlisteners can now override an effective message action, allowing the structured list result to remain tagged asmetadata; matching and telemetry continue to use the originally emitted action. Focused IO snapshots protect both output formats and listener tests cover emitted-versus-effective action behavior.Validation:
yarn compile,yarn eslint, and focused Jest coverage for the metadata command,CliIoHost, andIoHostRecorder(90 tests).Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license