Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Initialize/Shutdown on provider registration #449

Closed
lopitz opened this issue May 25, 2023 · 1 comment · Fixed by #456
Closed

Implement Initialize/Shutdown on provider registration #449

lopitz opened this issue May 25, 2023 · 1 comment · Fixed by #456
Assignees
Labels

Comments

@lopitz
Copy link
Contributor

lopitz commented May 25, 2023

I'd like to take over the task to implement initialize and shutdown on a provider switch, like mentioned here.

It currently looks to me pretty straight forward but please let me know, if you find some important points I might miss.

One of them could be exception handling, as this is not mentioned in the spec at all. Hence, I would go with logging only for exceptions to fulfill Requirement 1.4.9.

Another one could be execution time of the calls to the provider. I see 3 options, sync without timeout, sync with timeout and async.

Async

Advantages

  • setter immediately returns
  • allows for long initialization phase (e.g. while startup) to run in parallel w/o impacting start up time

Disadvantages

  • sdk code more complex as sdk needs to track initialization status of handler (esp. if handler is changed again, while the other handler is still initializing)
  • expectation of client code might be to be able to immediately evaluate flags, which is only possible after initialization (see Requirement 1.1.2.2)

Sync

Advantages

  • sdk code very easy

** Disadvantages **

  • clients may expect an immediate return of the provider mutator, as it is just a "setter"
  • "setter" method (provider mutator) potentially takes very long or even doesn't return at all, depending on the provider's initialization
  • in such a case, the application wouldn't work, just because feature flags are not available - which sounds to me like being the opposite of the whole sdk's design

Sync with Timeout

** Advantages**

  • "setter" method (provider mutator) would definitely return in a given timeframe

** Disadvantages **

  • sdk code more complex as timeout needs to be implemented
  • execution of provider mutator can still take very long (depending on the defined timeout)
  • what happens, if the mutator ran into the timeout? client not being informed, as no exception is thrown and hence no evaluation can be done

My initial thoughts were using the simple sync approach, but when writing up, I'd rather go with the async method as it allows for a "very long" initialization of the provider w/o impacting the client code.

cc @open-feature/sdk-java-approvers @open-feature/sdk-java-maintainers

@toddbaert
Copy link
Member

toddbaert commented May 25, 2023

Excellent write-up. I had similar thoughts. The web-sdk (and PRs for the js-sdk) implements what you refer to as async. For the async proposal to be properly usable, events really need to be implemented as well, since the READY event fires once the initialization is complete, which informs application authors (via their client handlers) that they can start evaluating flags.

In fact, because provider configuration and flag evaluation are generally happening in "different parts" of the code (and quite likely different threads in Java) events are how we expect evaluation code to be informed of the status of the provider; this is true not matter which option we choose above.

I also think Async is probably best here, but Sync with timeout also seems reasonable.

If you feel so inclined, you can also take on events since these enhancements are tightly coupled. It's not necessary though. I can implement events separately after your work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants