Add Prioritized Priority Consumer Policy#965
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request implements support for the "prioritized" priority policy for JetStream consumers, enabling message delivery prioritization based on a specified priority value (0-9, with 0 being highest priority).
- Adds "prioritized" as a valid priority policy option alongside existing "overflow" and "pinned_client" policies
- Introduces Priority field to consumer request models and fetch options for specifying delivery priority
- Updates consumer validation logic and includes comprehensive unit tests for the new functionality
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/NATS.Client.JetStream.Tests/PriorityGroupTest.cs | Adds unit tests for prioritized policy functionality and validation |
| src/NATS.Client.JetStream/NatsJSOpts.cs | Adds Priority property to NatsJSPriorityGroupOpts for specifying message priority |
| src/NATS.Client.JetStream/NatsJSContext.Consumers.cs | Updates validation to accept "prioritized" policy and improves error message |
| src/NATS.Client.JetStream/NatsJSConsumer.cs | Passes Priority value from fetch options to consumer requests |
| src/NATS.Client.JetStream/Models/ConsumerGetnextRequest.cs | Adds Priority field to consumer request model with validation |
| src/NATS.Client.JetStream/Models/ConsumerConfig.cs | Updates documentation to mention "prioritized" policy |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| if (config.PriorityPolicy is "pinned_client") | ||
| { | ||
| throw new NotImplementedException("Pinned clients are not supported yet."); |
There was a problem hiding this comment.
pinned_client wan't implemented anyway, so just removed it here.
| } | ||
|
|
||
| // TODO: enum these values? | ||
| if (config.PriorityPolicy != null && config.PriorityPolicy != "none" && config.PriorityPolicy != "overflow" && config.PriorityPolicy != "pinned_client") |
There was a problem hiding this comment.
don't need this check any more. it's an enum now.
|
|
||
| /// <summary> | ||
| /// Specifies the priority policy for consumer message selection, such as prioritizing <c>overflow</c> or <c>pinned_client</c>. | ||
| /// Specifies the priority policy for consumer message selection. |
There was a problem hiding this comment.
no need for documenting values. it's an enum now. it's a breaking change but since this is a fairly new feature, i don't expect many applications would be affected by it. required recompilation but the fix should be straight forward.
There was a problem hiding this comment.
Yeah, this is fine. I've done the same on brand new things.
| } | ||
|
|
||
| [Fact] | ||
| public void ConsumerConfigPriorityPolicy_null_test() |
There was a problem hiding this comment.
we also have a test for null where it should not be serialized onto the wire at all when null.
# Conflicts: # src/NATS.Client.JetStream/NatsJSJsonSerializer.cs # tests/NATS.Client.JetStream.Tests/EnumJsonTests.cs
|
|
||
| /// <summary> | ||
| /// Specifies the priority policy for consumer message selection, such as prioritizing <c>overflow</c> or <c>pinned_client</c>. | ||
| /// Specifies the priority policy for consumer message selection. |
There was a problem hiding this comment.
Yeah, this is fine. I've done the same on brand new things.
| [System.Text.Json.Serialization.JsonPropertyName("priority")] | ||
| [System.Text.Json.Serialization.JsonIgnore(Condition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingDefault)] | ||
| [System.ComponentModel.DataAnnotations.Range(0, 9)] | ||
| public byte Priority { get; set; } |
There was a problem hiding this comment.
when these are serialized, are the names lowercased?
probably what this is for
new JsonStringEnumConverter<ConsumerConfigPriorityPolicy (JsonNamingPolicy.SnakeCaseLower),
There was a problem hiding this comment.
I assume None does not get written either.
This pull request adds support for the "prioritized" priority policy for JetStream consumers as described in ADR-42, enabling message delivery prioritization based on a specified priority value. It updates the consumer configuration, request models, and fetch options to include a
Priorityfield used when the "prioritized" policy is set. The changes are validated and tested with new unit tests.Breaking change:
PriorityPolicyon consumer config is now implemented as anenuman it would break applications using this field asstringbefore. Applications not using this field won't be affected.Support for prioritized priority policy:
PriorityPolicyinConsumerConfig, and updated validation to accept it. [1] [2]Priorityproperty toConsumerGetnextRequestandNatsJSPriorityGroupOpts, allowing clients to specify delivery priority (0-9, with 0 as highest). [1] [2]NatsJSConsumerto pass thePriorityvalue from fetch options into the consumer request. [1] [2] [3]Testing and validation: