Polly
Polly copied to clipboard
Extract static SystemClock to interface
The issue or feature being addressed
I'd like to propose extracting static SystemClock to an interface.
Details on the issue fix or feature implementation
These are initial commits with the end goal to eventually get rid of the ambient SystemClock instance by allowing specifying one when the policies gets created. This will allow to run all tests in parallel and will restrict rogue system clock injection during runtime.
Confirm the following
- [x] I started this PR by branching from the head of the latest dev vX.Y branch, or I have rebased on the latest dev vX.Y branch, or I have merged the latest changes from the dev vX.Y branch
- [x] I have targeted the PR to merge into the latest dev vX.Y branch as the base branch
- [x] I have included unit tests for the issue/feature
- [x] I have successfully run a local build
@eugbaranov Thanks for this! We considered doing this couple years ago around #359 for the reasons you mention; great to have the reminder.
Any thoughts around syntax options for configuring the over-ridden clock on policies? I want to avoid new overloads of the already-over-proliferated, existing policy configuration overload format. I do plan to make a new syntax proposal which would make policy configuration syntax (generally) more elegant. Can imagine we might have choices for over-riding the policy clock like property-injection, extension method, or via the new policy builder syntax.
The usage here means we'd need to take a breaking-change on ITtlStrategy, so the transition to policy-scoped clocks would probably be part of a major release.
Linking also: #156
Avoiding new overloads, I think the easiest would be to have a new method which you could use to set non-standard system clock, e.g. Policy.WithSystemClock(new TestSystemClock()).Timeout(30).
Is this using ISystemClock by chance?
Apologies for not updating this PR sooner - I was trying to implement new WithSystemClock syntax but it turned out to be trickier than I've anticipated.
@jjxtra Polly is not using any ISystemClock from the base class libraries (was that the question?). Polly has a SystemClock class which aggregates a few time-related helpers. EDIT: We may split these out also if we move to an injectible form.
@eugbaranov No problem. It may be even easier to switch to an injectible clock (with nice syntax) when the syntax changes proposed here are enacted. (Cannot confirm at this stage, but work on this may happen as part of v8 this spring).
@reisenberger the proposed syntax looks like a good improvement however since I'm adding clock to the PolicyBuilder I found policies not using that to be the biggest challenge, e.g. Policy.Cache - at the moment is returns CachePolicy.
Ideally I wouldn't like to use something like Policy.WithClock(testClock).Cache(...) but I suppose Policy.Cache(builder => builder.WithClock(testClock).CacheProvider(...)).
@eugbaranov Yes, exactly - that's exactly the idea.
Closing due to inactivity.
Please open a new pull request if you would like to continue with a proposal for this change which we could consider for a v8 of Polly.