Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces an enabled flag to the publish hedging options, allowing users to explicitly enable or disable hedging. It updates the default options, the HedgingOptions and ResolvedHedgingOptions interfaces, and the validation logic to respect this flag. It also adds corresponding unit and system tests. The feedback suggests making the enabled property in ResolvedHedgingOptions required rather than optional, as it is always returned as true when the options are successfully resolved.
| * Validated and defaulted hedging configuration. | ||
| */ | ||
| export interface ResolvedHedgingOptions { | ||
| readonly enabled?: boolean; |
There was a problem hiding this comment.
Since validateAndResolveHedgingOptions always returns { enabled: true, ... } when hedging is enabled, and returns undefined otherwise, the enabled property in ResolvedHedgingOptions will always be present and true. It should be defined as a required boolean (readonly enabled: boolean;) rather than optional to avoid unnecessary null/undefined checks for consumers of ResolvedHedgingOptions.
| readonly enabled?: boolean; | |
| readonly enabled: boolean; |
c7afb42 to
0ad47ac
Compare
8f26d64 to
1bfd5e3
Compare
Overview
This is a tentative standalone (unstacked) catch-all PR on top of Stack #9467 (#9462-#9466, #9635-#9636) for post-merge adjustments to Publish Hedging in
@google-cloud/pubsub.Summary of Changes in This PR
HedgingOptions.enabled(handwritten/pubsub/src/publisher/hedging.ts,handwritten/pubsub/src/default-options.ts):enabled?: booleantoHedgingOptionsanddefaultOptions.publish.hedging.enabled = true, aligning with the cross-language hedging settings surface (enabled: true, hedgeDelay: 1000ms (1s), maxTokens: 50, refillRatio: 0.1with bounds100ms..10s,1..250,0.001..0.2).undefinedfromHedgingTokenBucket.validateAndResolveHedgingOptionswhenhedging.enabled === falseallows callers to explicitly disable hedging via{enabled: false}.handwritten/pubsub/test/publisher/hedging.ts,handwritten/pubsub/system-test/hedging.test.ts):hedging: {enabled: false}disables hedging andhedging: {}defaultsenabledtotrue.