Adds debouncing FileSystemWatcher events #129
Conversation
…libabries, not the actual connection to the database)
|
I had to catch SQL timeout exceptions in SqlClientApp in a failing unit test to please the build pipeline. |
| return !string.IsNullOrEmpty(client.ServerVersion); | ||
| } | ||
| } | ||
| catch (SqlException ex) when (ex.Number == -2) // -2 means SQL timeout |
There was a problem hiding this comment.
Why did the SqlException start happening with this change? Was it consistent, or just a flaky test?
There was a problem hiding this comment.
Dunno, the build pipleline failed: https://github.com/natemcmaster/DotNetCorePlugins/runs/486084609
I'm not sure if it has been a transient error.
There was a problem hiding this comment.
I haven't seen it before. This test isn't using reloading though, so it's probably not related to this change.
|
|
||
| namespace McMaster.NETCore.Plugins.Internal | ||
| { | ||
| internal class Debouncer : IDisposable |
There was a problem hiding this comment.
Can we add some unit tests for this to verify its behavior (and prevent regressions in the future)?
There was a problem hiding this comment.
I've pushed some unit tests. Do you think they're sufficient?
I've noticed your comment in
PluginLoader:This PR is my proposal for a fix. It introduces a debouncer component to decouple
FileSystemWatcher's events. The default delay is 200 msecs, but you can configure it withPluginConfig.ReloadDelay. If you think another default value is reasonable, feel free to change it.