consider correcting typing of LDOptions's bootstrap

[jmhodges-color]

Is this a support request?
No

Describe the bug
The actual bug and request for a change is a lil cryptic, but let me state it and then explain the details

Would it be possible to get a more accurate type for bootstrap in LDOptions to help guide folks to correctly interpret the output of the Python SDK (and I imagine other server-side SDK's) FeatureFlagsState?

LDOption's bootstrap has the type LDFlagSet. The docs for the Python SDK's FeatureFlagsState says its JSON format is suitable for passing into bootstrap. However, the types of bootstrap and FeatureFlagsState are actually incompatible in a way that effects customers trying to allow for local development. The LDFlagSet is type as, essentially, a keyed object, while FeatureFlagsState's JSON is an Array at its top with (possibly) multiple items in it.

They happen to work okay because the JS code here wriggles around the problem because LDFlagSet's type is loose enough to allow it.

However, when you go to solve problems like those raised by needing to write our camelCase'ing flag names to create fake LDClients to allow engineers to do local development without needing to share a single LaunchDarkly environment and project, you hit the problem.

We use an HTTP endpoint in our server in local dev that uses the PythonSDK's all_flags_state (that returns a FeatureFlagsState) to bootstrap from a local flag data file without needing to contact LaunchDarkly. That's fetch on loading of the React app to fill in the faked out Provider and Client we have. But when you try to create the LDFlagKeyMap type and others needed by those Providers, you hit the problem that the FeatureFlagsState's JSON format is actually an Array with an undefined accounting for each of the possible number of items in it.

I'm assuming that the first item is a LDFlagSet (we make the simplifying assumption that all the items in the array, but that's likely not true), but I'm not sure if that's true or not.

To reproduce
Try to use a Python (or similar server-side client's) all_flags_state data to create your own no-request-making Client (and react Provider) by handling camelCasing and all that.

[jmhodges-color]

(This, of course, would also be mooted by the JS and React SDK's providing fakes for local development that don't contact LaunchDarkly)

[kinyoklion]

Hello @jmhodges-color,

Did you use the to_json_dict method of the FeatureFlagsState? This is supposed to produce output which is compatible with the JS SDK.

Returns a dictionary suitable for passing as JSON, in the format used by the LaunchDarkly
        JavaScript SDK. Use this method if you are passing data to the front end in order to
        "bootstrap" the JavaScript client.

Thank you,
Ryan

[eli-darkly]

To be clear, all of the server-side SDKs that have an "all flags" feature are supposed to produce JSON in the same format that the JS SDK understands; that's a primary use case for the feature, and it's intended to be consistent across platforms. So if there's any inconsistency there, it would be either a bug in the server-side SDK (in this case Python) or some kind of usage error.

It might be worth re-filing this issue in https://github.com/launchdarkly/python-server-sdk, since unless I've misunderstood what you wrote, this sounds more like a Python issue and we would want to know if other Python developers have run into it. It'd also be helpful to know how you are producing the JSON output (like are you using jsonpickle, json.dumps, etc.).

[jmhodges-color]

I we're in agreement that all the server-side SDKs generating the same stuff. What I'm saying is the type of bootstrap needs to change because it doesn't represent what's actually be passed to LDOptions's bootstrap (because it can take the old style of LDFlags but also support the new bootstrap format that Python and others create now). The LDFlags type for bootstrap is just-so-happening to work because it's a very vague type.

[eli-darkly]

@jmhodges-color: I want to make sure we really are talking about the same thing. The JSON representation of the Python SDK's FeatureFlagsState object should look more or less like this:

{
  "flagkey1": "value1",
  "flagkey2": "value2",
  "$flagsState": {
    "flagkey1": {
      "variation": 0
    },
    "flagkey2": {
      "variation": 1
    }
  },
  "$valid": true
}

There could be more properties within each object, and of course the flag keys and values will be different, but the intended format is still that there's one big JSON object containing a flag value for each flag key, plus two special keys $flagsState and $valid.

So, the reason I'm spelling all this out is that your original message said "FeatureFlagsState's JSON is an Array at its top with (possibly) multiple items in it." I don't mean to nitpick, but an array and an object are two very different things in JSON; the intended format does not include any arrays, and if you're seeing one, something is wrong. And, if you're seeing something that looks significantly different from what I wrote above, then please provide an example of what it looks like.

But, if what you're seeing does look like my example— see my next comment.

[eli-darkly]

Now, if the reference to arrays was just a mistake, and what you're seeing is really a JSON object that looks like my example, then I'm guessing that the unexpected data you saw was the two metadata properties $flagsState and $valid. Technically, the presence of those keys does not violate the definition of the LDFlagsSet type in TypeScript; they are just keys, and the type is defined as a map of keys to values of any type.

It's true that those two are not flag keys, so if you are trying to interpret this data with an assumption that every key is a flag key, that would be a problem. We don't advise trying to interpret that data— the purpose of that JSON representation is to be consumed by the JS SDK, which is why its exact schema isn't documented, because we don't recommend for applications to rely on such implementation details that are subject to change. But if you do want to interpret it, there's a simple rule of thumb you can use to avoid wrongly thinking something is a flag key: if it starts with $, then it is not a flag key. LaunchDarkly doesn't allow that character to ever be in a flag key— that's a documented restriction at the product level.

It'd be reasonable to wonder why we used this schema, rather than just doing something more straightforward like:

{
  "valid": true,
  "flags": {
    "flagkey1": {
      "value": "value1",
      "variation": 0
    },
    "flagkey2": {
      "value": "value2",
      "variation": 1
    }
  }
}

The answer to that is simply backward compatibility. Older versions of SDKs did not provide any of the metadata that those two special keys represent; the only information about a flag was its value, and so the format of the bootstrap data was defined as a map with a single value for each key, rather than an object with multiple properties like value and variation for each key. In order to extend this in a way that would allow newer server-side SDK versions to pass additional metadata to newer JS SDK versions, but that would still allow older SDK versions to interact with newer ones without completely breaking, we had to leave the "flagkey": "value" key-value pairs where they were, as top-level properties, and add the new information as new properties that could not overlap with any real flag key.

Unfortunately, TypeScript is not well suited to describing a schema with the pattern "it's an object where optional property X has a value of type T, and optional property Y has a value of type U, but any properties whose names are not X or Y are flag keys that map to flag values." But, again, these details were not really intended to be inspected programmatically by application code. It's a schema for communication between two LaunchDarkly SDKs, and its awkward shape was a necessary compromise due to the inherent compatibility issues in any such communication.

[jmhodges-color]

This was definitely our bug! Our team added some ( around some multi-line calls but did so in the context of a Python dict literal instead of the right-hand side of an assignment. In the dict literal, the the ( ... ) was interpreted as a single-item tuple instead of "do all these things and return the normal bit".