Added support direct get batch API #680
Conversation
* Added batch get contract
* Added implementation GetBatchDirectAsync
* Added eod parse in headers
* Modify StreamMsgBatchGetRequest
* Added tests for directGetBatch
* Changed tests, set SkipIfNatsServer * Applied dotnet format
|
|
|
adr https://github.com/nats-io/nats-architecture-and-design/blob/main/adr/ADR-31.md should be stable enough now we can move on with the implementation. also java implementation can be used as reference: nats-io/nats.java#1239 use of MessageBatchGetRequest |
|
(in case the edit above is missed) |
| { | ||
| ValidateStream(); | ||
|
|
||
| return _context.Connection.RequestManyAsync<StreamMsgBatchGetRequest, T>( |
There was a problem hiding this comment.
we should inline the RequestMany method here and handle error conditions as well.
| /// <param name="includeEob"><c>true</c> to send the last empty message with eobCode in the header; otherwise <c>false</c></param> | ||
| /// <param name="cancellationToken">A <see cref="CancellationToken"/> used to cancel the API call.</param> | ||
| /// <exception cref="InvalidOperationException">There was an issue, stream must have allow direct set.</exception> | ||
| public IAsyncEnumerable<NatsMsg<T>> GetBatchDirectAsync<T>(StreamMsgBatchGetRequest request, INatsDeserialize<T>? serializer = default, bool includeEob = false, CancellationToken cancellationToken = default) |
There was a problem hiding this comment.
we probably shouldn't expose includeEob since it's kind of internal. Do you see that being used somehow?
There was a problem hiding this comment.
Am I understanding correctly that we don't want to send eodCode related messages to the client?
Similar functionality is available in nats.java
There was a problem hiding this comment.
Not if there is a use case for it but as long as it defaults to false I'm ok with it.
| [JsonPropertyName("seq")] | ||
| [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] | ||
| [Range(ulong.MinValue, ulong.MaxValue)] | ||
| public ulong MinSequence { get; set; } |
There was a problem hiding this comment.
I think these names might've been update. Also inline with StreamMsgGetRequest we are calling this property Seq
https://github.com/nats-io/nats-architecture-and-design/blob/main/adr/ADR-31.md#request
There was a problem hiding this comment.
StreamMsgBatchGetRequest updated
* Rename StreamMsgBatchGetRequest
* Added specialized methods
* Code style
|
thanks @Ivandemidov00 👍 will check soon |
* Added throw exception
Note
Resolves #636