search

open-feature / dotnet-sdk

.NET implementation of the OpenFeature SDK
https://openfeature.dev
Apache License 2.0
61 stars 17 forks source link

[BUG] Obsolete synchronous `SetProvider` methods does not await async call #248

Closed jenshenneberg closed 5 months ago

jenshenneberg commented 5 months ago

Observed behavior

When calling the obsolete marked SetProvider methods here and here, the provider is not set as the async call to the underlying repository here and here is not awaited.

Expected Behavior

The SetProvider methods to set the provider.

Steps to reproduce

  1. Call SetProvider from synchronous code
  2. Observer provider has not been set.
jenshenneberg commented 5 months ago

I do not know if this is a problem all the time or if it's due to something else in my code base. Happy to add a unit test / minimal example, but I was wondering if the best approach would be to delete the two obsolete methods in question instead?

toddbaert commented 5 months ago

Hey @jenshenneberg, thanks. If I'm understanding you correctly, this is working as we'd expect (though admittedly it's not very ergonomic). SetProvider is not meant to block until the provider is initialized. The intended use of this method is to also add a READY event handler to the client or the API, which will fire after the provider is initialized and set.

We understand this is cumbersome, which is why this method is deprecated and we recommend using SetProviderAsync. I think in addition to the deprecation, we should have some better inline doc to help people understand how SetProvider can be used. It could be argued we could set the provider immediately and not await it's initialization, but I'm not sure such a change would be of much value since we intend to remove this method anyway; but I'm interested in your feedback.

This method is really "leftovers" from older versions of the OpenFeature spec, and will be removed in 2.0 of this SDK now that we've improved our ergonomics around provider initialization.

jenshenneberg commented 5 months ago

Hi @toddbaert. Thanks for your explanation - I didn't know of that nuance of the API. As the methods are destined for removal anyway, I'll close this ticket. Thanks 😃