search

microsoft / reverse-proxy

A toolkit for developing high-performance HTTP reverse proxy applications.
https://microsoft.github.io/reverse-proxy
MIT License
8.43k stars 828 forks source link

Use System.Text.Json instead of Json.Net in Operator Framework #874

Open galvesribeiro opened 3 years ago

galvesribeiro commented 3 years ago

Hello folks!

Was looking at the Operator Framework and realized it is using Json.Net.

Is there any particular reason we can't go with System.Text.Json?

I could make a PR if that is acceptable.

Thanks!

samsp-msft commented 3 years ago

The operator framework is an internal library that we have taken (temporary) custody of from another team that had developed it but ended up going a different direction. Its therefore not had the level of scrutiny that we'd do for code that is in .NET.

There are internal discussions about where it should live and which team should own it, so knowing who wants to use it and in what scope (as part of YARP or other projects) is definitely useful information that can help inform decisions.

Thankyou for filing an issue rather than just submitting a PR - as we don't yet know the direction we'll go with the library, I think we'll be a bit more cautious in what PRs we take to it - but switching from Json.Net to System.Text.JSON is pretty uncontroversial so @galvesribeiro go for it.

samsp-msft commented 3 years ago

Tagging @jkotalik for FYI

jkotalik commented 3 years ago

We'd definitely be willing to swap to System.Text.Json. We'd gladly accept a contribution.

karelz commented 3 years ago

Triage: Yes, we would take the contribution. Are you still interested? Or should we unassign for now?

dpbevin commented 2 years ago

1642 removes most usages of Json.Net package. Some minor cleanup remaining in:

The Yarp.Kubernetes.Controller module indirectly pulls in the Newtonsoft library via: KubernetesClient -> Microsoft.Rest.ClientRuntime -> Newtonsoft.Json. The good news is that the latest version of the KubernetesClient package removes the dependency on Microsoft.Rest.ClientRuntime, so once we update the KubernetesClient version we should be good.