fix(cli): cdk list pollutes stdout with synth and dependency status lines#1663
fix(cli): cdk list pollutes stdout with synth and dependency status lines#1663sai-ray wants to merge 6 commits into
cdk list pollutes stdout with synth and dependency status lines#1663Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
| // In CI, info-level output (incl. the synth-time line, I1000) routes to stdout next to the | ||
| // listing and breaks parsers. With --json, stdout is an explicit machine-readable contract. | ||
| // In both cases, suppress I1000 so stdout carries only the listing. | ||
| if (options.json || this.ioHost.isCI) { |
There was a problem hiding this comment.
Just disable them for any case please. Let us not get confused by reports saying this happens in CI. It happens everywhere.
There was a problem hiding this comment.
Converted to draft while I rework this.
The current approach raises the log level to warn during listing. It works, but it's pretty blunt, it mutates shared ioHost state and ends up swallowing other info output during the call too.
So I was thinking of giving the "Including dependency stacks" line its own message code. Then I'd suppress that and the synth-time line by code on the list path. Feels self contained and matches what we already do. Does that sound right to you, would you go about it differently?
| // listing and breaks parsers. With --json, stdout is an explicit machine-readable contract. | ||
| // In both cases, suppress I1000 so stdout carries only the listing. | ||
| if (options.json || this.ioHost.isCI) { | ||
| this.ioHost.once(IO.CDK_TOOLKIT_I1000, () => ({ preventDefault: true })); |
There was a problem hiding this comment.
I am CERTAIN there is a second message we need to silence as well.
mrgrain
left a comment
There was a problem hiding this comment.
I struggle understanding what we are trying to achieve here. Can you please structure the PR to show relevant screenshots of the pre #1603 output (have one for every mode you are interested in) and than include screenshots of today's output.
This way it will be abundantly clear which messages got introduced and how this PR is fixing it.
| this.ioHost.once(IO.CDK_TOOLKIT_I1000, () => ({ preventDefault: true })); | ||
| // Only the listing should reach stdout; drop info status lines (synth time, dependency notes). | ||
| const previousLevel = this.ioHost.logLevel; | ||
| this.ioHost.logLevel = 'warn'; |
There was a problem hiding this comment.
this doesn't seem like the right approach
There was a problem hiding this comment.
reworked to suppress both lines by message code (no gate). Second message was the dependency line, registered it in #1677 and suppressed here. Updated the PR description with pre and post screenshots.
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
cdk lsprints status lines like✨ Synthesis time: X.XXsandIncluding dependency stacks: ...alongside the stack listing. In CI these go to stdout, next to the listing, which breaks scripts that parse the output (issue aws/aws-cdk#38165).#1637 tried to fix this by suppressing the synth-time message under
--json, but plaincdk lsin CI was still affected, and the dependency line was never handled.Both lines now get suppressed on the list path so stdout is only the listing. The dependency line got a message code in #1677 so it can be targeted by code, same as the synth-time line:
This applies in all cases, not just CI or
--json.Before:
After:
Tests
cdk lsin CI produces only the stack listing, no status lines.Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license