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

Set timeout from init_config in requests wrapper as default #4226

Merged
merged 1 commit into from
Jul 30, 2019
Merged

Set timeout from init_config in requests wrapper as default #4226

merged 1 commit into from
Jul 30, 2019

Conversation

ChristineTChen
Copy link
Contributor

@ChristineTChen ChristineTChen commented Jul 29, 2019 •
edited
Loading

What does this PR do?

Sets the default field to timeout from init_config if it exists.

Motivation

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

Review checklist (to be filled by reviewers)

  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached
  • Feature or bugfix must have tests
  • Git history must be clean
  • If PR adds a configuration option, it must be added to the configuration file.

@ChristineTChen ChristineTChen requested review from a team as code owners July 29, 2019 22:36
@ChristineTChen ChristineTChen changed the title Add default timeout from init_config in requests wrapper Set timeout from init_config in requests wrapper as default Jul 29, 2019
ofek
ofek approved these changes Jul 30, 2019
View reviewed changes
datadog_checks_base/tests/test_http.py
@@ -60,6 +60,13 @@ def test_config_multiple_timeouts(self):

assert http.options['timeout'] == (10, 4)

def test_config_init_config_override(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

🔥

@codecov
Copy link

codecov bot commented Jul 30, 2019

Codecov Report

Merging #4226 into master will decrease coverage by 4.06%.
The diff coverage is 100%.

Impacted file tree graph

Impacted Files Coverage Δ
...adog_checks_base/datadog_checks/base/utils/http.py 100% <100%> (ø) ⬆️
datadog_checks_base/tests/test_http.py 98.2% <100%> (+0.01%) ⬆️
...checks_dev/tests/plugin/test_environment_runner.py
riakcs/datadog_checks/riakcs/riakcs.py
oracle/tests/test_oracle.py
...r/datadog_checks/openstack_controller/__about__.py
twistlock/datadog_checks/twistlock/twistlock.py
linkerd/datadog_checks/linkerd/linkerd.py
...oller/datadog_checks/openstack_controller/utils.py
...hecks/openstack_controller/openstack_controller.py
... and 710 more

6 similar comments
@codecov
Copy link

codecov bot commented Jul 30, 2019

Codecov Report

Merging #4226 into master will decrease coverage by 4.06%.
The diff coverage is 100%.

Impacted file tree graph

Impacted Files Coverage Δ
...adog_checks_base/datadog_checks/base/utils/http.py 100% <100%> (ø) ⬆️
datadog_checks_base/tests/test_http.py 98.2% <100%> (+0.01%) ⬆️
...checks_dev/tests/plugin/test_environment_runner.py
riakcs/datadog_checks/riakcs/riakcs.py
oracle/tests/test_oracle.py
...r/datadog_checks/openstack_controller/__about__.py
twistlock/datadog_checks/twistlock/twistlock.py
linkerd/datadog_checks/linkerd/linkerd.py
...oller/datadog_checks/openstack_controller/utils.py
...hecks/openstack_controller/openstack_controller.py
... and 710 more

@codecov
Copy link

codecov bot commented Jul 30, 2019

Codecov Report

Merging #4226 into master will decrease coverage by 4.06%.
The diff coverage is 100%.

Impacted file tree graph

Impacted Files Coverage Δ
...adog_checks_base/datadog_checks/base/utils/http.py 100% <100%> (ø) ⬆️
datadog_checks_base/tests/test_http.py 98.2% <100%> (+0.01%) ⬆️
...checks_dev/tests/plugin/test_environment_runner.py
riakcs/datadog_checks/riakcs/riakcs.py
oracle/tests/test_oracle.py
...r/datadog_checks/openstack_controller/__about__.py
twistlock/datadog_checks/twistlock/twistlock.py
linkerd/datadog_checks/linkerd/linkerd.py
...oller/datadog_checks/openstack_controller/utils.py
...hecks/openstack_controller/openstack_controller.py
... and 710 more

@codecov
Copy link

codecov bot commented Jul 30, 2019

Codecov Report

Merging #4226 into master will decrease coverage by 4.06%.
The diff coverage is 100%.

Impacted file tree graph

Impacted Files Coverage Δ
...adog_checks_base/datadog_checks/base/utils/http.py 100% <100%> (ø) ⬆️
datadog_checks_base/tests/test_http.py 98.2% <100%> (+0.01%) ⬆️
...checks_dev/tests/plugin/test_environment_runner.py
riakcs/datadog_checks/riakcs/riakcs.py
oracle/tests/test_oracle.py
...r/datadog_checks/openstack_controller/__about__.py
twistlock/datadog_checks/twistlock/twistlock.py
linkerd/datadog_checks/linkerd/linkerd.py
...oller/datadog_checks/openstack_controller/utils.py
...hecks/openstack_controller/openstack_controller.py
... and 710 more

@codecov
Copy link

codecov bot commented Jul 30, 2019 •
edited
Loading

Codecov Report

Merging #4226 into master will decrease coverage by 4.06%.
The diff coverage is 100%.

Impacted file tree graph

Impacted Files Coverage Δ
...adog_checks_base/datadog_checks/base/utils/http.py 100% <100%> (ø) ⬆️
datadog_checks_base/tests/test_http.py 98.2% <100%> (+0.01%) ⬆️
...checks_dev/tests/plugin/test_environment_runner.py
riakcs/datadog_checks/riakcs/riakcs.py
oracle/tests/test_oracle.py
...r/datadog_checks/openstack_controller/__about__.py
twistlock/datadog_checks/twistlock/twistlock.py
linkerd/datadog_checks/linkerd/linkerd.py
...oller/datadog_checks/openstack_controller/utils.py
...hecks/openstack_controller/openstack_controller.py
... and 710 more

@codecov
Copy link

codecov bot commented Jul 30, 2019

Codecov Report

Merging #4226 into master will decrease coverage by 4.06%.
The diff coverage is 100%.

Impacted file tree graph

Impacted Files Coverage Δ
...adog_checks_base/datadog_checks/base/utils/http.py 100% <100%> (ø) ⬆️
datadog_checks_base/tests/test_http.py 98.2% <100%> (+0.01%) ⬆️
...checks_dev/tests/plugin/test_environment_runner.py
riakcs/datadog_checks/riakcs/riakcs.py
oracle/tests/test_oracle.py
...r/datadog_checks/openstack_controller/__about__.py
twistlock/datadog_checks/twistlock/twistlock.py
linkerd/datadog_checks/linkerd/linkerd.py
...oller/datadog_checks/openstack_controller/utils.py
...hecks/openstack_controller/openstack_controller.py
... and 710 more

@codecov
Copy link

codecov bot commented Jul 30, 2019 •
edited
Loading

Codecov Report

Merging #4226 into master will decrease coverage by 4.06%.
The diff coverage is 100%.

Impacted file tree graph

Impacted Files Coverage Δ
...adog_checks_base/datadog_checks/base/utils/http.py 100% <100%> (ø) ⬆️
datadog_checks_base/tests/test_http.py 98.2% <100%> (+0.01%) ⬆️
...checks_dev/tests/plugin/test_environment_runner.py
riakcs/datadog_checks/riakcs/riakcs.py
oracle/tests/test_oracle.py
...r/datadog_checks/openstack_controller/__about__.py
twistlock/datadog_checks/twistlock/twistlock.py
linkerd/datadog_checks/linkerd/linkerd.py
...oller/datadog_checks/openstack_controller/utils.py
...hecks/openstack_controller/openstack_controller.py
... and 710 more

AlexandreYang
AlexandreYang reviewed Jul 30, 2019
View reviewed changes
datadog_checks_base/datadog_checks/base/utils/http.py
@@ -103,6 +103,7 @@ def __init__(self, instance, init_config, remapper=None, logger=None):
# Update the default behavior for global settings
default_fields['log_requests'] = init_config.get('log_requests', default_fields['log_requests'])
default_fields['skip_proxy'] = init_config.get('skip_proxy', default_fields['skip_proxy'])
default_fields['timeout'] = init_config.get('timeout', default_fields['timeout'])
Copy link
Member

Choose a reason for hiding this comment

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

Would that make sense to also add connect_timeout and read_ timeout to init config ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be addressed in a later PR that would set default_fields for all config options to the init_config value if it exists.

@ChristineTChen ChristineTChen mentioned this pull request Jul 30, 2019
Merged
6 tasks
@ChristineTChen ChristineTChen merged commit 4b9a3d0 into master Jul 30, 2019
@ChristineTChen ChristineTChen deleted the christine/http-add-init-timeout branch July 30, 2019 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexandreYang AlexandreYang AlexandreYang approved these changes

@hithwen hithwen hithwen approved these changes

@ofek ofek ofek approved these changes

Assignees
No one assigned
Labels
integration/datadog_checks_base
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ChristineTChen @hithwen @ofek @AlexandreYang