Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(config): warn for unknown config fields #267

Merged
merged 2 commits into from
Aug 1, 2024
Merged

fix(config): warn for unknown config fields #267

merged 2 commits into from
Aug 1, 2024

Conversation

aasseman
Copy link
Contributor

Fixes #259

@aasseman aasseman added p0 Critical priority type:bug Something isn't working labels Jul 31, 2024
@aasseman aasseman self-assigned this Jul 31, 2024
@aasseman aasseman changed the title test(config): unknown env vars w/ matching prefix fix(config): warn instead of crash for unknown fields Jul 31, 2024
@aasseman aasseman changed the title fix(config): warn instead of crash for unknown fields fix(config): warn for unknown config fields Aug 1, 2024
@aasseman aasseman force-pushed the aasseman/fix-259 branch 2 times, most recently from 36d5c1f to 872aae8 Compare August 1, 2024 02:18
@coveralls
Copy link

Pull Request Test Coverage Report for Build 10190770646

Details

  • 10 of 10 (100.0%) changed or added relevant lines in 1 file are covered.
  • 29 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.02%) to 66.118%
Files with Coverage Reduction New Missed Lines %
common/src/tap/checks/deny_list_check.rs 1 93.64%
tap-agent/src/agent/sender_account.rs 4 81.59%
config/src/config.rs 24 76.03%
Totals Coverage Status
Change from base Build 10188231836: 0.02%
Covered Lines: 3893
Relevant Lines: 5888
💛 - Coveralls

@aasseman aasseman marked this pull request as ready for review August 1, 2024 02:54
@aasseman aasseman requested a review from gusinacio August 1, 2024 02:54
@aasseman
Copy link
Contributor Author

aasseman commented Aug 1, 2024

First commit is to reproduce the issue, second one is the fix.

gusinacio
gusinacio approved these changes Aug 1, 2024
View reviewed changes
Copy link
Member

@gusinacio gusinacio left a comment

Choose a reason for hiding this comment

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

LGTM

@aasseman aasseman merged commit 45bddd5 into main Aug 1, 2024
19 checks passed
@aasseman aasseman deleted the aasseman/fix-259 branch August 1, 2024 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gusinacio gusinacio gusinacio approved these changes

Assignees

@aasseman aasseman

Labels
p0 Critical priority type:bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Config fails if unknown env var with the right prefix exists
3 participants
@aasseman @coveralls @gusinacio