Skip to content
This repository has been archived by the owner on Jun 2, 2023. It is now read-only.

Make streaming opt-in, not opt-out #56

Open
goncalossilva opened this issue Jan 27, 2022 · 5 comments
Open

Make streaming opt-in, not opt-out #56

goncalossilva opened this issue Jan 27, 2022 · 5 comments
Labels
client-backend Specific to the backend client client-frontend Specific to the frontend client question Further information is requested

Comments

@goncalossilva
Copy link
Member

goncalossilva commented Jan 27, 2022

Most teams at Doist disable streaming to ensure each individual session is stable, without flag values changing at runtime. This runs counter to the current SDK default, which is to stream. Should the default be inverted?

@goncalossilva goncalossilva added question Further information is requested client-frontend Specific to the frontend client client-backend Specific to the backend client labels Jan 27, 2022
@goncalossilva
Copy link
Member Author

@pixyzehn @panpeter @gnapse thoughts?

@gnapse
Copy link

gnapse commented Jan 28, 2022

I did not even know this was a thing. Indeed, I assumed that feature flag values remained stable, and only changed on fresh page loads. We actually even cache feature flag values in memory.

I do not have strong opinions on what the default should be, as long as this setting can be set opposite to the default.

@pixyzehn
Copy link
Member

pixyzehn commented Jan 28, 2022

From my experience on iOS, I expected that streaming is disabled unless we explicitly set it to enabled. I'm curious why the current SDK enables it by default (they even changed so starting from v5, ref), but I believe streaming can be enabled explicitly and when needed as it can be harmful and unexpected.

Another approach I see in LaunchDarkly is to allow us to subscribe flag changes for each flag, which can be flexible and explicit to developers.

@panpeter
Copy link

I think it's simpler if the flags are frozen and don't change values at runtime. Otherwise, as you mentioned, it's possible that the app become unstable. For example, before having frozen flags, we had one flag value when rendering UI, and then it was possible to have a different value in a view model. Obviously, that caused issues.

So I think the default should be that the streaming is disabled.

@goncalossilva
Copy link
Member Author

The default should be the common case, so let's change it to not stream. Thanks!

I'm curious why the current SDK enables it by default

Good question. I thought I was following the standard approach, but I don't know where I got that idea from. Probably, too small of a sample size. 😅

Another approach I see in LaunchDarkly is to allow us to subscribe flag changes for each flag, which can be flexible and explicit to developers.

We can add this if it's widely useful. It would not be a complex addition.

@goncalossilva goncalossilva changed the title Should streaming be enabled by default? Make streaming opt-in, not opt-out Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
client-backend Specific to the backend client client-frontend Specific to the frontend client question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants