-
Notifications
You must be signed in to change notification settings - Fork 126
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: convert DAPR_HEALTH_TIMEOUT to float #692
Conversation
3f7c7c0
to
3c08c46
Compare
Signed-off-by: KentHsu <chiahaohsu9@gmail.com>
3c08c46
to
2243cbe
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #692 +/- ##
==========================================
- Coverage 86.37% 86.28% -0.09%
==========================================
Files 79 82 +3
Lines 4094 4172 +78
==========================================
+ Hits 3536 3600 +64
- Misses 558 572 +14 ☔ View full report in Codecov by Sentry. |
tests/clients/test_heatlhcheck.py
Outdated
@@ -62,7 +62,7 @@ def test_wait_until_ready_success_with_api_token(self, mock_urlopen): | |||
self.assertIn('Dapr-api-token', headers) | |||
self.assertEqual(headers['Dapr-api-token'], 'mytoken') | |||
|
|||
@patch.object(settings, 'DAPR_HEALTH_TIMEOUT', 2) | |||
@patch.object(settings, 'DAPR_HEALTH_TIMEOUT', '2') |
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.
Let's make this 2.5, so it's more explicit.
@patch.object(settings, 'DAPR_HEALTH_TIMEOUT', '2') | |
@patch.object(settings, 'DAPR_HEALTH_TIMEOUT', '2.5') |
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.
updated in this commit
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.
Signed-off-by: KentHsu <chiahaohsu9@gmail.com>
Thank you for reviewing this PR @elena-kolevska |
Thanks! LGTM now. |
Description
Convert settings.DAPR_HEALTH_TIMEOUT from str to float
Update DaprHealthCheckTests mock so the test can catch this error
Issue reference
Fixes #688 [BUG] DAPR_HEALTH_TIMEOUT as environment variable is not converted to numerical value
Please reference the issue this PR will close: #[#688]
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: