-
Notifications
You must be signed in to change notification settings - Fork 193
feat: ability to configure the min size for response compression #2060
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: ability to configure the min size for response compression #2060
Conversation
|
""" WalkthroughThe changes introduce a configurable parameter for the minimum response size required to enable HTTP response compression in the router. This replaces the previously hardcoded 4KB threshold with a value that can be set in the router's configuration files and schema. A new test verifies compression behavior with different minimum size settings. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
router/pkg/config/config.schema.json (1)
1472-1476: Consider addingbytes-stringformat & a sane minimum to tighten validationThe new
response_compression_min_sizefield currently omits the custombytes-stringformat and has an emptybytesobject, meaning any string – including empty or invalid values like"foobar"or"0"– will pass schema validation.
Other byte-size properties in the schema (e.g.persisted_operations.cache.size) use the format and often specify a minimum threshold, which catches typos early and keeps UX consistent."response_compression_min_size": { "type": "string", + "format": "bytes-string", "description": "The minimum size of the response body in bytes to enable response compression. The size is specified as a string with a number and a unit, e.g. 10KB, 1MB, 1GB. The supported units are 'KB', 'MB', 'GB'.", "default": "4KB", - "bytes": {} + "bytes": { + "minimum": "1B" + } }Adding these two lines will align validation behaviour with neighbouring fields and prevent silent mis-configurations.
Please double-check that the Go struct tag inconfig.gostill uses the same default after the change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
router/core/graph_server.go(1 hunks)router/pkg/config/config.go(1 hunks)router/pkg/config/config.schema.json(1 hunks)router/pkg/config/fixtures/full.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: SkArchon
PR: wundergraph/cosmo#1929
File: router/internal/circuit/manager.go:16-25
Timestamp: 2025-06-30T20:39:02.387Z
Learning: In the Cosmo router project, parameter validation for circuit breaker configuration is handled at the JSON schema level rather than through runtime validation methods on structs. The config.schema.json file contains comprehensive validation constraints for circuit breaker parameters.
router/pkg/config/config.go (1)
Learnt from: endigma
PR: wundergraph/cosmo#2009
File: router/pkg/config/config.go:0-0
Timestamp: 2025-07-03T10:33:25.778Z
Learning: The CardinalityLimit field in the Metrics struct (router/pkg/config/config.go) is validated at the JSON schema level in config.schema.json with a minimum value constraint of 1, preventing zero or negative values without requiring runtime validation.
router/pkg/config/config.schema.json (2)
undefined
<retrieved_learning>
Learnt from: endigma
PR: #2009
File: router/pkg/config/config.go:0-0
Timestamp: 2025-07-03T10:33:25.778Z
Learning: The CardinalityLimit field in the Metrics struct (router/pkg/config/config.go) is validated at the JSON schema level in config.schema.json with a minimum value constraint of 1, preventing zero or negative values without requiring runtime validation.
</retrieved_learning>
<retrieved_learning>
Learnt from: SkArchon
PR: #1929
File: router/internal/circuit/manager.go:16-25
Timestamp: 2025-06-30T20:39:02.387Z
Learning: In the Cosmo router project, parameter validation for circuit breaker configuration is handled at the JSON schema level rather than through runtime validation methods on structs. The config.schema.json file contains comprehensive validation constraints for circuit breaker parameters.
</retrieved_learning>
🧬 Code Graph Analysis (1)
router/pkg/config/config.go (1)
router/pkg/config/marshaler.go (1)
BytesString(54-54)
🔇 Additional comments (2)
router/pkg/config/fixtures/full.yaml (1)
197-202: Fixture looks good – confirms schema updateThe sample now includes
response_compression_min_size: 4KB, matching the new struct field and preserving the previous hard-coded default.
Nothing to change here.router/core/graph_server.go (1)
286-286: LGTM! Clean implementation of configurable response compression minimum size.The change successfully replaces the hardcoded 4KB threshold with a configurable value from the router traffic configuration. The type conversion to
intis appropriate for thegzhttp.MinSizefunction parameter.
…ia an environment variable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also add the original default value to this function
func DefaultRouterTrafficConfig() *config.RouterTrafficConfiguration {
Add ability to configure the min size for response compression.
Checklist
Resolve issue: #2056
Documentation update: wundergraph/cosmo-docs#124
Summary by CodeRabbit