Avoid fetching feature again in GetVariant

[jameshartig]

Avoid fetching feature again in GetVariant

[jameshartig]

If you're generally okay with the proposed solution I'm fine submitting a PR myself.

[thomasheartman]

Hey, @jameshartig! Thanks for the issue. I know you've got a few of them open (this one, #162, and #160), so permit me to briefly talk to you here for some info. Are they all still relevant after your merging of #164?

Regarding this one specifically:

For clarity, do you mean that whenever you call GetVariant, the SDK makes an API call to Unleash to fetch the updated state about the flag? All of Unleash's SDKs poll Unleash at set intervals, so you can make as many checks to isEnabled you want without making any extra network calls. So the impact of doing this should at most be that you can skip some in-memory evaluation, I would think. However, I haven't looked that deeply into this SDK myself; are you saying that that's not how it works?

[jameshartig]

Thanks for the issue. I know you've got a few of them open (this one, #162, and #160), so permit me to briefly talk to you here for some info. Are they all still relevant after your merging of #164?

Yes, #160 is a bug with either the documentation or in the code but I'm not sure which. #162 is a feature request.

For clarity, do you mean that whenever you call GetVariant, the SDK makes an API call to Unleash to fetch the updated state about the flag? All of Unleash's SDKs poll Unleash at set intervals, so you can make as many checks to isEnabled you want without making any extra network calls. So the impact of doing this should at most be that you can skip some in-memory evaluation, I would think. However, I haven't looked that deeply into this SDK myself; are you saying that that's not how it works?

Yes, I understand it doesn't do another network lookup, but it does call r.options.storage.Get again which happens to be Redis for us. Let me create a quick PR to show my proposed solution since I think will be simpler to get across.

[thomasheartman]

Great; thanks! 🙏🏼

It appears that #162 has been resolved and that you have a PR in the works for this one that @FredrikOseberg has said he'll have a look at, so that's sorted.

Regarding #160, I think it's the documentation that's unclear (but I'll double check against the implementations in the other SDKs). And while I think you're right in that it's at least misleading, it could be read as being correct: if the flag exists, then you'll always find a variant (even if it's the disabled variant), so the only time the sdk won't be able to find the variant is if the flag is missing. However, that is not at all obvious and the names can certainly be misleading. I'll add it as a thing to solve and see if we can tackle it shortly.