Flagsmith / flagsmith-dotnet-client

.NET Standard Client for Flagsmith. Ship features with confidence using feature flags and remote config. Host yourself or use our hosted version at https://www.flagsmith.com/
https://www.flagsmith.com/
BSD 3-Clause "New" or "Revised" License
19 stars 12 forks source link

chore(engine): add missing model properties #68

Closed luk355 closed 1 year ago

luk355 commented 1 year ago

Add missing properties to engine model so that engine flagState result can be mapped to API /identities response.

matthewelwell commented 1 year ago

Hey @luk355, we might need to rethink this one since I believe this will break local evaluation mode. The environment document is currently built without the id (and then stored in our DynamoDB database so it would be a painful migration to add it). As far as I'm aware the deserialization would therefore fail if the id doesn't exist, although I'm confused why the tests have passed on this PR. I would have expected the tests to fail at this point, perhaps there's something I'm missing in the way in which .net deserializes data? Is it not strict perhaps?

Aside from the above, I'm also not keen on diverging the engine schemas in our server side SDKs. Is there another way around this problem?

If it's helpful, you can see an example of an environment document here.

luk355 commented 1 year ago

Fair enough. Thanks for the comment. I have found a way how to lookup data from the DB model so this PR is not needed.