NehanPathan commented on code in PR #1170:
URL: https://github.com/apache/lucenenet/pull/1170#discussion_r2303279153
##########
src/Lucene.Net.Replicator/SessionToken.cs:
##########
@@ -112,6 +114,33 @@ public void Serialize(DataOutputStream writer)
}
}
+ /// <summary>
+ /// Asynchronously serialize the token data for communication between
server and client.
+ /// </summary>
+ /// <param name="output">The <see cref="Stream"/> to write the token
data to.</param>
+ /// <param name="cancellationToken">A cancellation token to observe
while waiting for the flush to complete.</param>
+ /// <returns>A task representing the asynchronous operation.</returns>
+ public async Task SerializeAsync(Stream output, CancellationToken
cancellationToken = default)
+ {
+ using var writer = new DataOutputStream(output);
+ writer.WriteUTF(Id);
Review Comment:
@NightOwl888
---
I wanted to update you regarding the low-level UTF and integer stream
methods. I initially attempted to implement them using `Span<byte>` /
`ReadOnlySpan<byte>` to reduce allocations and align with BCL-style patterns.
However, I ran into a limitation: the async `Stream.ReadAsync` and
`Stream.WriteAsync` overloads that accept `Span<byte>` / `ReadOnlySpan<byte>`
**are only available in C# 13 / .NET 8+**. Since the current project targets
**C# 11 / .NET Standard 2.1**, the compiler throws errors like:
* `No overload for method 'WriteAsync' takes 2 arguments`
* `Cannot convert from 'Span<byte>' to 'Memory<byte>'`
* `Feature 'ref and unsafe in async and iterator methods' is not available
in C# 11.0`
Because of this, I propose we **use `byte[]` for all Read/Write methods**,
including the UTF methods, which keeps compatibility with the current project
and matches how other synchronous Read/Write helpers work.
The **current code is working completely fine and perfectly**, and we can
still include the updated UTF exception handling (`EncoderFallbackException` /
`DecoderFallbackException`) as per BCL conventions.
This approach ensures the code works across the current codebase without
forcing an upgrade to C# 13.
Let me know your thoughts with this approach, and I can finalize the changes
accordingly.
---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
