consuldotnet icon indicating copy to clipboard operation
consuldotnet copied to clipboard

Switch to System.Text.Json

Open mfkl opened this issue 2 years ago • 4 comments

Rebased and iterated on https://github.com/G-Research/consuldotnet/pull/73

Few points to discuss:

  • Bundled a few needed ConfigurationAwait(false) in the initial commit.
  • which version of Text.Json do we want? This comment is on point but over a year passed, so we can probably pick a reasonable version now.
  • Marcin dynamic comment,
  • do we want to keep a netcoreapp2.1 target? https://github.com/G-Research/consuldotnet/commit/f042debdb40692d3a8617f446f5b5d395553a3b5

Closes https://github.com/G-Research/consuldotnet/issues/43

mfkl avatar Jul 14 '22 15:07 mfkl

netcoreapp2.1 failures are expected but there is a net461 timing one.

[xUnit.net 00:01:01.68]     Consul.Test.LockTest.Lock_ContendFast [FAIL]
Error: Consul.Test.LockTest.Lock_ContendFast: Consul.ConsulRequestException : Unexpected response, status code InternalServerError: invalid session "3103bd01-8165-4b14-cddd-542796ae036a"
  Failed Consul.Test.LockTest.Lock_ContendFast [31 s]
  Error Message:
   Consul.ConsulRequestException : Unexpected response, status code InternalServerError: invalid session "3103bd01-8165-4b14-cddd-542796ae036a"
  Stack Trace:
    at Consul.PutRequest`2[TIn,TOut].Execute (System.Threading.CancellationToken ct) [0x002c5] in <04779d9f65704d909393c78b5c007b81>:0 
  at Consul.Lock.Acquire (System.Threading.CancellationToken ct) [0x00447] in <04779d9f65704d909393c78b5c007b81>:0 
  at Consul.Lock.Acquire (System.Threading.CancellationToken ct) [0x00797] in <04779d9f65704d909393c78b5c007b81>:0 
  at Consul.Test.LockTest+<>c__DisplayClass9_1.<Lock_ContendFast>b__0 () [0x00096] in <3c10[17](https://github.com/G-Research/consuldotnet/runs/7343250916?check_suite_focus=true#step:9:18)3e055c4e98a841ccf76066[21](https://github.com/G-Research/consuldotnet/runs/7343250916?check_suite_focus=true#step:9:22)5d>:0 
  at Consul.Test.TimeoutUtils.WithTimeout (System.Threading.Tasks.Task task) [0x000fe] in <3c10173e055c4e98a841ccf76066215d>:0 
  at Consul.Test.LockTest.Lock_ContendFast () [0x000d8] in <3c10173e055c4e98a841ccf76066215d>:0

mfkl avatar Jul 14 '22 16:07 mfkl

If we want to bump the minor version to 1.7 for this change, we may want to provide bindings for all 1.7 Consul APIs, such as Namespaces https://github.com/G-Research/consuldotnet/issues/117. This needs a consul enterprise version, so I reached out to Consul for it. Let's see.

mfkl avatar Aug 23 '22 10:08 mfkl

We can probably continue with this.

winstxnhdw avatar Aug 31 '23 11:08 winstxnhdw

We can probably continue with this.

@winstxnhdw Feel free to give it a try.

marcin-krystianc avatar Sep 01 '23 12:09 marcin-krystianc