Skip to content

feat(config): Add Validation to Several Config Fields#664

Merged
george-sentry merged 4 commits into
mainfrom
george/feat/config-validation
Jun 2, 2026
Merged

feat(config): Add Validation to Several Config Fields#664
george-sentry merged 4 commits into
mainfrom
george/feat/config-validation

Conversation

@george-sentry

@george-sentry george-sentry commented Jun 2, 2026

Copy link
Copy Markdown
Member

Linear

Refs STREAM-843

Description

Some configuration options have specific requirements, like they must be greater than zero or a power of two. Until now, we've relied on ad hoc assertions and calls to functions like max(1) to enforce these constraints. I think it would be better to guarantee these constraints immediately on startup because ad hoc checks can easily be forgotten.

@george-sentry george-sentry requested a review from a team as a code owner June 2, 2026 14:15

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ebd653a. Configure here.

Comment thread src/config.rs

@untitaker untitaker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validation annotations are good, please make sure the design is still consistent with #663 where we have another validation method on Config

@untitaker

untitaker commented Jun 2, 2026

Copy link
Copy Markdown
Member

one thing to think about: impl Validate for Config inherently exposes a public validation function. is there a way to make it private (and call it in validate_and_normalize), or maybe validate_and_normalize can be made private and called via this validate crate?

https://github.com/Keats/validator/blob/master/README.md#struct-level-validation

@linear-code

linear-code Bot commented Jun 2, 2026

Copy link
Copy Markdown

STREAM-843

@george-sentry george-sentry merged commit adff79b into main Jun 2, 2026
25 checks passed
@george-sentry george-sentry deleted the george/feat/config-validation branch June 2, 2026 17:24
Comment thread src/config.rs
// Use "__" for nested configurations via environment variables, like `TASKBROKER_KAFKA_TOPICS__PROFILES__CLUSTER`
builder = builder.merge(Env::prefixed("TASKBROKER_").split("__"));
let mut config: Config = builder.extract()?;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@george-sentry I think we should not have two of these functions. normalize_and_validate is called from tests, and in those tests we should call validate too IMO

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants