flowmade-one/aether
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[Issue #38] Make configuration values injectable rather than hardcoded #43

Merged
HugoNijhuis merged 1 commits from issue-38-injectable-config into main 2026-01-10 15:10:50 +00:00
Conversation 1 Commits 1 Files Changed 13 +353 -74
HugoNijhuis commented 2026-01-10 14:34:15 +00:00
Owner
Copy Link

Closes #38

Summary

  • Added JetStreamConfig struct with configurable StreamRetention (default: 1 year) and ReplicaCount (default: 1)
  • Added HashRingConfig struct with configurable VirtualNodes (default: 150)
  • Added ShardConfig struct with configurable ShardCount (default: 1024) and ReplicationFactor (default: 1)
  • Each component has a new WithConfig constructor while maintaining backward compatibility with original constructors
  • Zero values in configs fall back to defaults for backward compatibility

Test plan

  • All existing tests pass with default configuration
  • New tests verify custom configuration works correctly
  • Tests verify zero-value configs use defaults
  • go build ./... and go vet ./... pass

Generated with Claude Code

Closes #38 ## Summary - Added `JetStreamConfig` struct with configurable `StreamRetention` (default: 1 year) and `ReplicaCount` (default: 1) - Added `HashRingConfig` struct with configurable `VirtualNodes` (default: 150) - Added `ShardConfig` struct with configurable `ShardCount` (default: 1024) and `ReplicationFactor` (default: 1) - Each component has a new `WithConfig` constructor while maintaining backward compatibility with original constructors - Zero values in configs fall back to defaults for backward compatibility ## Test plan - [x] All existing tests pass with default configuration - [x] New tests verify custom configuration works correctly - [x] Tests verify zero-value configs use defaults - [x] `go build ./...` and `go vet ./...` pass Generated with Claude Code
HugoNijhuis added 1 commit 2026-01-10 14:34:15 +00:00
Make configuration values injectable rather than hardcoded
All checks were successful
CI / build (pull_request) Successful in 16s
Details
CI / build (push) Successful in 15s
Details
c757bb76f3 
Add config structs with sensible defaults for tunable parameters:
- JetStreamConfig for stream retention (1 year) and replica count (1)
- HashRingConfig for virtual nodes per physical node (150)
- ShardConfig for shard count (1024) and replication factor (1)

Each component gets a new WithConfig constructor that accepts custom
configuration, while the original constructors continue to work with
defaults. Zero values in configs fall back to defaults for backward
compatibility.

Closes #38

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
HugoNijhuis commented 2026-01-10 15:10:45 +00:00
Author
Owner
Copy Link

AI Code Review

This is an automated review generated by the code-reviewer agent.

Summary

This PR successfully makes configuration values injectable across JetStream, HashRing, and ShardManager components while maintaining full backward compatibility. The implementation is clean, well-tested, and follows Go conventions.

Findings

Code Quality

  • Excellent: Config structs are well-designed with clear field names and comments
  • Good practice: Zero-value fallback pattern (if config.Field == 0, use default) ensures backward compatibility
  • Consistent: All three components (JetStream, HashRing, ShardManager) follow the same config pattern
  • Clean: Added getter methods (GetVirtualNodes, GetShardCount, GetReplicationFactor) provide safe access to internal fields
  • Minor formatting: Good cleanup of whitespace and alignment issues throughout

Potential Bugs

  • No issues found. The implementation correctly handles:
    • Zero-value configs falling back to defaults
    • Legacy constructors still working via delegation to new config-based constructors
    • All edge cases covered in tests

Security Concerns

  • No security concerns identified. Configuration changes are purely structural and don't introduce new attack surfaces.

Style Notes

  • Consistent with codebase conventions
  • Good use of constants (DefaultVirtualNodes, DefaultNumShards, etc.) instead of magic numbers
  • Proper documentation comments for exported functions and types
  • The rename from NumShards/VirtualNodes to DefaultNumShards/DefaultVirtualNodes improves clarity

Test Coverage

  • Excellent coverage: New tests verify:
    • Custom configuration values work correctly
    • Zero-value configs use defaults
    • Legacy constructors still work
    • Different shard counts produce valid results
  • Tests in cluster/config_test.go and store/config_test.go are thorough and well-structured
  • All existing tests still pass, confirming backward compatibility

Verdict

LGTM

This PR achieves its goal of making configuration injectable while maintaining full backward compatibility. The code is well-tested, follows best practices, and integrates cleanly with the existing codebase. Ready to merge.

## AI Code Review > This is an automated review generated by the code-reviewer agent. ### Summary This PR successfully makes configuration values injectable across JetStream, HashRing, and ShardManager components while maintaining full backward compatibility. The implementation is clean, well-tested, and follows Go conventions. ### Findings #### Code Quality - **Excellent**: Config structs are well-designed with clear field names and comments - **Good practice**: Zero-value fallback pattern (if config.Field == 0, use default) ensures backward compatibility - **Consistent**: All three components (JetStream, HashRing, ShardManager) follow the same config pattern - **Clean**: Added getter methods (GetVirtualNodes, GetShardCount, GetReplicationFactor) provide safe access to internal fields - **Minor formatting**: Good cleanup of whitespace and alignment issues throughout #### Potential Bugs - No issues found. The implementation correctly handles: - Zero-value configs falling back to defaults - Legacy constructors still working via delegation to new config-based constructors - All edge cases covered in tests #### Security Concerns - No security concerns identified. Configuration changes are purely structural and don't introduce new attack surfaces. #### Style Notes - Consistent with codebase conventions - Good use of constants (DefaultVirtualNodes, DefaultNumShards, etc.) instead of magic numbers - Proper documentation comments for exported functions and types - The rename from NumShards/VirtualNodes to DefaultNumShards/DefaultVirtualNodes improves clarity #### Test Coverage - **Excellent coverage**: New tests verify: - Custom configuration values work correctly - Zero-value configs use defaults - Legacy constructors still work - Different shard counts produce valid results - Tests in cluster/config_test.go and store/config_test.go are thorough and well-structured - All existing tests still pass, confirming backward compatibility ### Verdict **LGTM** This PR achieves its goal of making configuration injectable while maintaining full backward compatibility. The code is well-tested, follows best practices, and integrates cleanly with the existing codebase. Ready to merge.
HugoNijhuis merged commit c757bb76f3 into main 2026-01-10 15:10:50 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
risk/high
risk/low
risk/medium
value/critical
value/high
value/low
value/medium
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
HugoNijhuis fluxcd-bot renovate-bot
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: flowmade-one/aether#43
Delete Branch "issue-38-injectable-config"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?