CHANGE @W-17701097@ Logfile now communicated at start. #1745
Conversation
jfeingold35
force-pushed
the
d/W-17701097
branch
from
581c837 to
99d0bcb
Compare
jfeingold35
force-pushed
the
d/W-17701097
branch
from
99d0bcb to
e035132
Compare
messages/action-summary-viewer.md
Outdated
| @@ -1,3 +1,7 @@ | |||
| # common.streaming-logs-to | |||
|
|
|||
| **Streaming logs in real time to:** | |||
There was a problem hiding this comment.
@jshackell-sfdc , Net-new message. Example of its use can be seen in the PR's description section.
There was a problem hiding this comment.
LGTM (although I assume you'll remove the ** around the message? Or do you intend to leave that? It looks odd to me...)
|
|
||
| // We always add a Logger Listener to the appropriate listeners list, because we should always be logging. | ||
| const logFileWriter: LogFileWriter = await LogFileWriter.fromConfig(userConfig); | ||
| this.dependencies.actionSummaryViewer.viewPreExecutionSummary(logFileWriter.getLogDestination()); |
There was a problem hiding this comment.
This is the functional change to this Action. I added it as early in the method as I could (i.e., immediately after the log file writer is instantiated) to minimize the possibility of it being preempted by an error.
If we want, though, we could easily move it further down and use it in the future to log additional information about, for example, the engines that are added. I'm flexible on this.
| public async execute(input: RulesInput): Promise<void> { | ||
| const config: CodeAnalyzerConfig = this.dependencies.configFactory.create(input['config-file']); | ||
| const logWriter: LogFileWriter = await LogFileWriter.fromConfig(config); | ||
| this.dependencies.actionSummaryViewer.viewPreExecutionSummary(logWriter.getLogDestination()); |
There was a problem hiding this comment.
This is the functional change to this Action. I added it as early in the method as I could (i.e., immediately after the log file writer is instantiated) to minimize the possibility of it being preempted by an error.
If we want, though, we could easily move it further down and use it in the future to log additional information about, for example, the engines that are added. I'm flexible on this.
| public async execute(input: RunInput): Promise<void> { | ||
| const config: CodeAnalyzerConfig = this.dependencies.configFactory.create(input['config-file']); | ||
| const logWriter: LogFileWriter = await LogFileWriter.fromConfig(config); | ||
| this.dependencies.actionSummaryViewer.viewPreExecutionSummary(logWriter.getLogDestination()); |
There was a problem hiding this comment.
This is the functional change to this Action. I added it as early in the method as I could (i.e., immediately after the log file writer is instantiated) to minimize the possibility of it being preempted by an error.
If we want, though, we could easily move it further down and use it in the future to log additional information about, for example, the engines that are added. I'm flexible on this.
| this.display = display; | ||
| } | ||
|
|
||
| public viewPreExecutionSummary(logFile: string): void { |
There was a problem hiding this comment.
The Display object isn't directly accessible to the Action layer, so I opted to add the pre-execution output as another capability of the ActionSummaryViewer family instead of giving the Actions a new Dependency.
| } | ||
|
|
||
| public view(logFile: string, outfile?: string): void { | ||
| public viewPostExecutionSummary(logFile: string, outfile?: string): void { |
There was a problem hiding this comment.
The concrete summary viewers' view method was renamed to viewPostExecutionSummary to distinguish it from the net-new viewPreExecutionSummary.
| public view(logFile: string, outfile?: string): void { | ||
| public viewPostExecutionSummary(logFile: string, outfile?: string): void { | ||
| // Start with separator to cleanly break from anything that's already been logged. | ||
| this.displayLineSeparator(); |
There was a problem hiding this comment.
This was missing the leading newline that all the other summary viewers have, so I went ahead and added it.
stephen-carter-at-sf
left a comment
stephen-carter-at-sf
left a comment
There was a problem hiding this comment.
Approved pending Juliet's approval.
Adds a new log event near the very beginning of action execution, which indicates where logs are being written to. This way, if the process appears to hang or is taking a long time, the user knows where to look for additional logging.
Example of this new message in use (the "Streaming logs in real time to:" message):