open-feature / ofep

A focal point for OpenFeature research, proposals and requests for comments
https://openfeature.dev
20 stars 15 forks source link

Provider to client mappings #56

Closed justinabrahms closed 1 year ago

justinabrahms commented 1 year ago

refs: #49

beeme1mr commented 1 year ago

@justinabrahms how would you feel about something like this?

import { OpenFeature } from '@openfeature/js-sdk';

OpenFeature.setProvider(new MyGlobalProvider());

const client = OpenFeature.getClient();
client.overrideProvider(new MyClientProvider());

The idea would be to only allow a single global provider at a time but allow clients to optionally set a new provider that only affects themselves.

justinabrahms commented 1 year ago

@beeme1mr A different way to say that:

  1. libraries must never create a client. They must be passed a client.
  2. ~If you need to juggle multiple providers (e.g. flagsmith for some stuff, flagd for others, file-based for a third), you will need to juggle multiple clients.~

~I don't love either of those.~

EDIT: Just realized that my solution does item 2 as well. So even if I don't love it.. it's what we got. The big question is item 1, then.

justinabrahms commented 1 year ago

All good here. Brings utility to the existing named clients (but also will be a breaking change, just to note).

My intent would be to still allow setProvider(provider) to set the default one, which should keep it backwards compatible, yes?

beeme1mr commented 1 year ago

Hey @justinabrahms, would you mind putting together a quick prototype when you have a moment? That may be a good way to move this OFEP along and would make updating the spec straightforward.

justinabrahms commented 1 year ago

Here's a prototype: https://github.com/open-feature/java-sdk/pull/388

toddbaert commented 1 year ago

Here's a prototype: open-feature/java-sdk#388

Thanks @justinabrahms I'll take a look at this tomorrow!

toddbaert commented 1 year ago

Here's a prototype: open-feature/java-sdk#388

@justinabrahms This makes sense to me. I have a couple thoughts/questions:

cc @thiyagu06 @tcarrio @beeme1mr @thomaspoignant

justinabrahms commented 1 year ago

@toddbaert I dislike using the term 'scope' for the client name. It introduces another term, which I think confuses things. What we're really talking about is binding a client to a specific provider. It's not some "scoping key" that is being bound. It's the named client.

I would expect that initialization would happen for any provider that's registered. I'm unclear about how events would be supported. One option would be a single event stream with a parameter of what is effected or we could have multiple event streams. I don't feel strongly.

I think API shutdown would be global.

thomaspoignant commented 1 year ago

I can agree with @justinabrahms I don't really like the word scope in this context it can be misleading and it is not completely obvious that we are talking about the client name.

toddbaert commented 1 year ago

I would expect that initialization would happen for any provider that's registered. I'm unclear about how events would be supported. One option would be a single event stream with a parameter of what is effected or we could have multiple event streams. I don't feel strongly.

@justinabrahms my main concern was to confirm that if provider associated with client X gets a "flags change" event, only client X would get that event, right?

justinabrahms commented 1 year ago

@toddbaert I'm not fully up to speed on the events stuff. I don't think there's a technical reason why the thing you're saying can't work though.