-
Notifications
You must be signed in to change notification settings - Fork 2
All CDK-Assets logging needs to be pluggable #196
Comments
Are you sure we need this? The /**
* Listener for progress events
*
* @default No listener
*/
readonly progressListener?: IPublishProgressListener; That is used by the CDK CLI. CDK CLI funnels log events to the regular CDK CLI logger. |
Great find! In that case we just need to make sure this is used everywhere. I came across the Docker asset building implementation which is not using the logger at the moment: cdk-assets/lib/private/shell.ts Line 38 in 79c76a7
|
I was discussing this with @HBobertz today, and we're not quite sure what the best solution is here. This is for build output that is intended primarily for a human's eyeballs, not for a program. Of course, in an IDE context or whatever, we would want to capture it so we can send it somewhere else. Buffering What do you think of the following approach:
|
Closes #196 Removes quiet flag and also ensures all events are routed to the progressListener. removeD the existing `logger` callback and replaces it with `shellEventPublisher` which will route the message back to the configured progressListener if configured. The default behavior is `stdio` for the new `subprocessOutputDestination` option, which essentially is the old behavior. Where shells write directly to stdout/stderr --------- Co-authored-by: Rico Hermans <rix0rrr@gmail.com>
Currently
cdk-assets
writes logs directly to stdout and stderr. It also uses a booleanquiet
flag to decide if output is printed or not.quiet
should be mapped to a log levelThe text was updated successfully, but these errors were encountered: