Expose the status of currently-running tasks #239
Conversation
There was a problem hiding this comment.
A few more nits, but I'm satisfied that the only major concern is the sticky API comment I left first.
Once some of my questions are answered, I can be more confident that I do understand each part, and I can probably test and approve quickly after that.
Awesome work! Super creative, idiomatic in a lot of ways, really good API decisions, and solid code.
| -- - warnings from outside of the error boundary do not impact the FailureBundles produced by the action | ||
| -- | ||
| -- This returns a FailureBundle if the action failed; otherwise returns the result of the action | ||
| errorBoundary :: Has Diagnostics sig m => m a -> m (Either FailureBundle a) |
There was a problem hiding this comment.
[no action, maybe?]: We probably need a guideline to decide when this is appropriate. Originally, the only way to "catch" was to use runDiagnostics or recover, and this provides another option. I think we should document or at least agree when each should/shouldn't be used.
There was a problem hiding this comment.
Agreed. I'll put together some docs. errorBoundary is primarily a hack, and is otherwise analogous to runDiagnostics.
errorBoundary is a workaround to allow us to send the Diagnostics constraint upward, so we can intercept context calls from the inner action. We use this for stickyDiag specifically
Using several incurs a minor performance overhead
| @@ -136,16 +137,18 @@ uploadBuildGraphChunk url httpOptions targets = do | |||
| _ <- sendIO $ runDiagnostics $ runHTTP $ req POST url (ReqBodyJson jsonChunk) ignoreResponse httpOptions | |||
| pure () | |||
|
|
|||
| updateProgress :: Has Logger sig m => Progress -> m () | |||
| updateProgress :: Has StickyLogger sig m => Progress -> m () | |||
There was a problem hiding this comment.
[future work]: Should we move this to the TaskPool modules? I see it everywhere, and I don't think there are any changes between implementations.
Fixes https://github.com/fossas/team-analysis/issues/418 . Details available in that ticket
The user-visible changes for this include:
Diagnosticscontextstack is visible at the bottom of the terminal for each running task (when the terminal is ANSI-compatible)--debugmode (very) noisily outputs eachcontextchange for each task as discrete log messages (this works even when the terminal is not ANSI-compatible)contextadded for:contextwhat step is currently running in the analyzerExeccontextfor running commandsReadFScontextfor parsing various types of filescontextfor walking the filetreeThis PR also addresses some tech debt:
concurrent-outputpackage for thislogWithExithelperData.String.ConversiondecodeUtf8functions and replaces them withdecodeUtf8With lenientDecodeResultBundlefromDiagnostics, as the warnings from the bundle were never actually used (and it just made for a worse dev experience)