FluxMonitor: Expose DeviationChecker concurrency#2259
Conversation
|
@felder-cl could you include a link to the story? |
Its not directly related to one. Its in support for upcoming changes for this one: |
There was a problem hiding this comment.
Why would we connect again if we are already connected?
There was a problem hiding this comment.
I'm not sure to be honest. This change preserves the same behavior though, so I assume its intended.
There was a problem hiding this comment.
The method name connectSingleChecker and the error message we returned before were a bit misleading, as they were really just a wrapper around checker.Start, which also contained what is now checker.Connect. Just reads weird now, worth renaming to make what's going on more clear.
There was a problem hiding this comment.
Does this panic if err? Seems like execution might continue here, which might be undesirable.
There was a problem hiding this comment.
No this just logs an error, it does not panic
There was a problem hiding this comment.
Is this mocked? If so, is there any point in connecting here?
There was a problem hiding this comment.
Its mocked out but instantiates fields within the struct. Start kind of assumes Connect was run.
There was a problem hiding this comment.
I'll clarify this in the code
Give the user of the DeviationChecker control over concurrency by splitting its Start(..) method into one which connects and reports errors, and a second for polling the endpoint metrics. This is in support avoiding race conditions in upcoming tests.
9cef642 to
4f30edd
Compare
|
Closing in favor of making Stop() wait in #2233. This is still broken and might be nice to revisit later. The caller should generally be given control over concurrency, methods should not return with backgrounded processes. |
|
@felder-cl doesn't |
If 'Start()' returns with backgrounded processes, then it would have been by mistake. Am I missing something? |
4 participants
Give the user of the DeviationChecker control over concurrency by splitting its Start(..) method into one which connects and reports errors, and a second for polling the endpoint metrics.
This is in support avoiding race conditions in upcoming tests.