Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support System.Text.Json.Utf8JsonReader #14

Closed
bartelink opened this issue Dec 30, 2018 · 3 comments
Closed

Support System.Text.Json.Utf8JsonReader #14

bartelink opened this issue Dec 30, 2018 · 3 comments
Assignees
Labels
help wanted Extra attention is needed
Milestone

Comments

@bartelink
Copy link
Collaborator

Main constraint on the impl is that we don't want to foist a netstandard2.1 dependency on anyone - the codec may hence need to be a separate project/nuget - benchmarks with the memory store should make sense as a way of validating any potential raw perf improvement.

For Equinox.EventStore, the wiring should be straightforward

Wrt Equinox.Cosmos, this could become more messy due to the way in which the data is emitted (and how the DocDb client dll wants to manage its parsing)

@bartelink
Copy link
Collaborator Author

https://github.com/Tarmil/FSharp.SystemTextJson is looking very promising - it has great test and benchmark coverage and features are being implemented at a dizzying rate; I'm thus strongly considering doing an Equinox.Codec.SystemTextJson nuget which would provide a drop-in Equinox.Codec.SystemTextJson.Json.Create (and would welcome with open arms the contribution of same!). I should point out that my hope is that eventually the stock System.Text.Json provide the bulk of its functionality [wrt first class F# support] out of the box); the point is that there's no longer a blocker feature wise in terms of getting something set up in terms of having to wait for someone to do something.

I'm open to less ugly name, i.e. maybe Equinox.Codec.SystemTextJson.Create ?. As with jet/equinox#147, if any rename (and knock-on-effects such as e.g. renaming Equinox.NewtonsoftJson.Json.Create to match) is to happen, it should happen before the stable v 2.0.0 release is minted.

A related note: While Equinox.Cosmos for now (and definitely in V2) will not yet depend on the cosmos v3 sdk, it seems serialization therein is going to be sufficiently pluggable in a 3.x release to enable an efficient binding. Using the v3 sdk may mean introducing an Equinox.Cosmos3 or Equinox.Cosmos2 nuget in order to let consumers pick which Cosmos SDK version they want to depend on (the v2 API is and has been seeing very minor traffic whereas one can see significant work and focus on the v3 repo with important features like observability getting focus).

@bartelink bartelink transferred this issue from jet/equinox Aug 29, 2019
@bartelink bartelink added this to the 3.0 milestone Jan 13, 2020
@bartelink
Copy link
Collaborator Author

Update:

  • FsCodec 2.0.0 still only implements newtonsoft.json
  • the obvious impl approach involves copying and porting impl+tests herein (targeting netcoreapp3.1/netstandard2.1 should be fine)
  • Equinox's master has an Equinox.Cosmos using the V3 SDK (which uses json.net same as V2 SDK does)
  • feat!(CosmosStore): Azure.Cosmos 'V4' support / enable mocking Cosmos APIs equinox#197 is currently also using json.net in the impl, but the intention is for that to start using STJ internally to produce/parse requests/response (which is orthogonal to whether the app level produces buffers with newtonsoft vs STJ)

@bartelink bartelink self-assigned this Mar 8, 2020
@bartelink bartelink added help wanted Extra attention is needed and removed up-for-grabs labels Mar 8, 2020
bartelink added a commit that referenced this issue May 10, 2020
@bartelink
Copy link
Collaborator Author

Implemented in #38; released in 2.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant