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.

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

Fix unhandled default params #1154

Merged
merged 1 commit into from
Jan 6, 2025
Merged

Conversation

gandarez
Copy link
Member

@gandarez gandarez commented Jan 6, 2025

This PR fixes unhandled default params when flag/config is not set. Currently any value informed from flag/config is completely ignored for rate limit and timeout. The vipertools.FirstNonEmptyInt() function now checks if value is set and accepts zero as valid one.

Some tests have been added to ensure the right behavior and increase coverage.

Fixes #1147

@gandarez gandarez requested a review from alanhamlett January 6, 2025 12:47
@gandarez gandarez self-assigned this Jan 6, 2025
@gandarez gandarez force-pushed the bugfix/unhandled-default-params branch 2 times, most recently from 63839eb to a254257 Compare January 6, 2025 12:50
@gandarez gandarez enabled auto-merge January 6, 2025 12:54
@gandarez gandarez disabled auto-merge January 6, 2025 14:01
@gandarez gandarez force-pushed the bugfix/unhandled-default-params branch from a254257 to 4f340c5 Compare January 6, 2025 15:47
@gandarez gandarez force-pushed the bugfix/unhandled-default-params branch from 4f340c5 to abcf3bc Compare January 6, 2025 15:49
@gandarez gandarez enabled auto-merge January 6, 2025 15:49
@gandarez gandarez force-pushed the bugfix/unhandled-default-params branch 2 times, most recently from 8deaf74 to 5a2d529 Compare January 6, 2025 16:09
@gandarez gandarez force-pushed the bugfix/unhandled-default-params branch from 5a2d529 to 51ab45b Compare January 6, 2025 16:14
@gandarez gandarez force-pushed the bugfix/unhandled-default-params branch from 51ab45b to 15f70f2 Compare January 6, 2025 16:17
@gandarez gandarez merged commit 02eefc9 into develop Jan 6, 2025
12 checks passed
@gandarez gandarez deleted the bugfix/unhandled-default-params branch January 6, 2025 16:22
Copy link

codecov bot commented Jan 6, 2025

Codecov Report

Attention: Patch coverage is 72.72727% with 12 lines in your changes missing coverage. Please review.

Project coverage is 62.99%. Comparing base (92addb5) to head (15f70f2).
Report is 2 commits behind head on develop.

Files with missing lines Patch % Lines
pkg/api/error.go 16.66% 10 Missing ⚠️
cmd/heartbeat/heartbeat.go 0.00% 2 Missing ⚠️
@@             Coverage Diff             @@
##           develop    #1154      +/-   ##
===========================================
+ Coverage    62.97%   62.99%   +0.02%     
===========================================
  Files          386      386              
  Lines        16887    16914      +27     
===========================================
+ Hits         10634    10655      +21     
- Misses        5679     5686       +7     
+ Partials       574      573       -1     
Flag Coverage Δ
unittests 62.99% <72.72%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
cmd/params/params.go 81.37% <100.00%> (+0.18%) ⬆️
pkg/api/heartbeat.go 80.00% <100.00%> (+0.22%) ⬆️
pkg/vipertools/vipertools.go 100.00% <100.00%> (ø)
cmd/heartbeat/heartbeat.go 76.67% <0.00%> (+1.48%) ⬆️
pkg/api/error.go 7.40% <16.66%> (+2.64%) ⬆️

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.

--heartbeat-rate-limit-seconds flag is not properly configured in the config file
2 participants