Closed toddbaert closed 2 months ago
After chewing on this for a night, I'm starting to feel:
I think that we should probably have tests which have a dummy async provider that blocks evaluations until cancelled, then we should start an evaluation and cancel it to make sure all the steps happen. Maybe something similar with a hook implementation.
That's a good idea. Will add.
I think that we should probably have tests which have a dummy async provider that blocks evaluations until cancelled, then we should start an evaluation and cancel it to make sure all the steps happen. Maybe something similar with a hook implementation.
That's a good idea. Will add.
I've added tests for cancellation (evaluation/resolution and hooks): https://github.com/open-feature/dotnet-sdk/pull/268/commits/036d7a45c5627c4fb17ce7ab7208cd52ca19b7ca
This PR is a combination of https://github.com/open-feature/dotnet-sdk/pull/184 and https://github.com/open-feature/dotnet-sdk/pull/185. Changes include:
setProviderAsync
,InitializeAsync
, I've specified in the in-line doc that the cancellation token's purpose is to cancel such side-effects - so setting a provider and canceling that operation still results in that provider's being set, but async side-effect should be cancelled. I'm interested in feedback here, I think we need to consider the semantics around this... I suppose the alternative would be to always ensure any state changes only occur after async side-effects, if they weren't cancelled beforehand.SetProvider
methodsValueTask
for hook methodsTasks
toValueTasks
, from the official .NET docs:ValueTask
especially makes sense since often hooks are synchronous, in fact async hooks are probably the less likely variant.Task
, but there could be an argument for making themValueTask
, since some providers resolve asynchronously.ValueTask
, so I'm really interested in feedback hereUPDATE:
After chewing on this for a night, I'm starting to feel: