-
Notifications
You must be signed in to change notification settings - Fork 631
[DEVOPS-1063] improve error message when a worker throws #3664
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use Text
instead of String
. Otherwise this looks good!
Let's squash those commits upon merging. We don't want to solve 4 times the same conflict when merging :) |
, useStackBinaries ? false | ||
# one of "nix", "stack" or "path" | ||
, binaryMethod ? "nix" | ||
, forceDontCheck ? false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
an unrelated change that got partially polled in by the cherry-pick, i could remove it from this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about the nix changes going on there?
They don't seem related to the workers change so I'd rather have them in a separated PR.
Also, a minor change on moving to add the plugin's name for readability. CI isn't cheap though so I might simply move them as part of another PR,,,
forever $ liftIO $ do | ||
newUpdate <- WalletLayer.waitForUpdate w | ||
logInfo "A new update was found!" | ||
WalletLayer.addUpdate w . cpsSoftwareVersion $ newUpdate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather have left this file untouched and simply add the workers' names when actually passing the "Plugin" in
(same for the Legacy ones, even though less important since they'll be removed soon enough)
4c86bbe
to
dc1096d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
[DEVOPS-1063] improve error message when a worker throws
…hk/devops-1063 [DEVOPS-1063] improve error message when a worker throws
(cherry picked from commit 3243271)
without these changes, you get:
[cardano-sl.reporter:Error:ThreadId 627] [2018-09-25 18:16:53.23 UTC] Reporting error with reason "Worker/plugin with logger name "*production*" failed with exception: state-wallet-mainnet-staging/tls/server/server.crt: openBinaryFile: does not exist (No such file or directory){ nodeIps: '[]', peers: 'OutboundQ internal state '{Peers {peersRoutes = Routes {_routesCore = [], _routesRelay = [], _routesEdge = []}, peersClassification = fromList []}}'' }"
with the changes, you get:
[cardano-sl.reporter:Error:ThreadId 627] [2018-09-25 18:16:53.23 UTC] Reporting error with reason "Worker/plugin with work name "doc worker" and logger name "*production*" failed with exception: state-wallet-mainnet-staging/tls/server/server.crt: openBinaryFile: does not exist (No such file or directory){ nodeIps: '[]', peers: 'OutboundQ internal state '{Peers {peersRoutes = Routes {_routesCore = [], _routesRelay = [], _routesEdge = []}, peersClassification = fromList []}}'' }"
it adds
work name "doc worker"
and now you know where to even look for a problemDescription
Linked issue
Type of change
Developer checklist
Testing checklist
QA Steps
Screenshots (if available)