Skip to content

Conversation

@klevy-toast
Copy link
Contributor

@klevy-toast klevy-toast commented Oct 17, 2025

Fixes #1429

Motivation

Use -1 as a sentinel value for all the namespace / topic admin get commands that return an empty body to mean "unset"

Modifications

-1 is the default return for the various get commands:

Namespace Admin Methods
- GetNamespaceMessageTTL / GetNamespaceMessageTTLWithContext
- GetMaxConsumersPerTopic / GetMaxConsumersPerTopicWithContext
- GetMaxProducersPerTopic / GetMaxProducersPerTopicWithContext
- GetMaxConsumersPerSubscription / GetMaxConsumersPerSubscriptionWithContext
- GetOffloadThreshold / GetOffloadThresholdWithContext
- GetOffloadThresholdInSeconds / GetOffloadThresholdInSecondsWithContext
- GetOffloadDeleteLag / GetOffloadDeleteLagWithContext
- GetCompactionThreshold / GetCompactionThresholdWithContext
Topic Admin Methods
- GetMessageTTL / GetMessageTTLWithContext
- GetMaxProducers / GetMaxProducersWithContext
- GetMaxConsumers / GetMaxConsumersWithContext
- GetMaxUnackMessagesPerConsumer / GetMaxUnackMessagesPerConsumerWithContext
- GetMaxUnackMessagesPerSubscription / GetMaxUnackMessagesPerSubscriptionWithContext
- GetCompactionThreshold / GetCompactionThresholdWithContext
- GetMaxConsumersPerSubscription / GetMaxConsumersPerSubscriptionWithContext
- GetMaxMessageSize / GetMaxMessageSizeWithContext
- GetMaxSubscriptionsPerTopic / GetMaxSubscriptionsPerTopicWithContext
- GetDeduplicationSnapshotInterval / GetDeduplicationSnapshotIntervalWithContext

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

(example:)

  • Added new tests for the various commands that were not already covered by tests

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (yes / no)
  • The public API: (yes / no)
  • The schema: (yes / no / don't know)
  • The default values of configurations: (yes / no)
  • The wire protocol: (yes / no)

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / GoDocs / not documented)
  • If a feature is not applicable for documentation, explain why?
  • If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

@klevy-toast klevy-toast force-pushed the fix/admin-unset-sentinel-values branch from d5f758e to dd5ad95 Compare October 17, 2025 06:53
@klevy-toast klevy-toast force-pushed the fix/admin-unset-sentinel-values branch from dd5ad95 to c91362e Compare October 17, 2025 16:48
@klevy-toast klevy-toast marked this pull request as ready for review October 20, 2025 17:36
@RobertIndie RobertIndie modified the milestones: v0.17.0, v0.18.0 Oct 23, 2025
@RobertIndie RobertIndie merged commit c0db482 into apache:master Oct 23, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Confusing handling of empty config values in admin client

2 participants