open-feature / spec

OpenFeature specification
https://openfeature.dev
Apache License 2.0
595 stars 35 forks source link

Specify provider state after `shutdown()` #213

Closed toddbaert closed 7 months ago

toddbaert commented 8 months ago

We don't currently specify the state of the provider after a shutdown().

I'm proposing we should explicitly require (MUST) or suggest (SHOULD) the provider transitions back into either NOT_READY (implying that it can be re-initialized) or READY (for a provider not requiring initialization) once it has been shut down. I think this is a fairly sensible approach, and will remove this ambiguity in a clean way. Some providers already work this way.

Essentially we'd be adding another transition in the state diagram here from READY to NOT_READY via shutdown.

kinyoklion commented 8 months ago

This effectively disallows providers that would hit some terminal state, they are neither ready, nor could they be initialized again. Potentially this is fine, but I think it is worth an explicit callout.

toddbaert commented 8 months ago

This effectively disallows providers that would hit some terminal state, they are neither ready, nor could they be initialized again. Potentially this is fine, but I think it is worth an explicit callout.

I agree, and think that might be optimal. If we go this route, we can note that. Our state diagrams will also be circular, which will communicate the same thing.

justinabrahms commented 8 months ago

+1

weyert commented 8 months ago

In my opinion shutdown is a final state so no need to recover from by going into a NOT_READY state. If they want to recover from it they can recreate the provider?

lukas-reining commented 8 months ago

I would be fine with either a terminal state or the requirement for all providers to be able to be reenitialized which would imply the two states READY or NOT_READY as you said @toddbaert. First I was leaning towards the terminal state but I think as every provider has to be initialized anyways, I would go for beeing able to reenitialize, which should be doable in any case (by just running everything from constructor and a full cleanup in the shutdown)

Kavindu-Dodan commented 8 months ago

I am voting for recommending going into a NOT_READY state.

When shutdown is called, the provider is meant to clean up its resources (as per spec The provider MAY define a mechanism to gracefully shutdown and dispose of resources. ). This clearly make them NOT_READY to serve flag evaluations. And the status changes beyond that are out of scope of the OF spec.

toddbaert commented 7 months ago

In my opinion shutdown is a final state so no need to recover from by going into a NOT_READY state. If they want to recover from it they can recreate the provider?

That was certainly the case before. The problem is the lack of any prescription here makes things ambiguous. It seems to me that we'd like to make the provider lifecycle as consistent as possible - it's already one of the trickiest areas for provider and SDK implementors.

I think we have majority support for shutdown transitioning back to NOT_READY, and a lot of providers behave this way already. I will open a spec PR for this.

toddbaert commented 7 months ago

@lukas-reining @kinyoklion @justinabrahms and others, please take a look at the associated spec PR if you get a sec: https://github.com/open-feature/spec/pull/216