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

Set requests stream option to false when scraping Prometheus endpoints #1596

Merged

Conversation

antoinepouille
Copy link
Contributor

@antoinepouille antoinepouille commented May 23, 2018

What does this PR do?

Change the stream parameter to the request to False, as we don't stream it.

Motivation

We can avoid here to use this option, as we only store the fetched value, and don't stream it.

Review checklist

  • PR has a meaningful title or PR has the no-changelog label attached
  • Feature or bugfix has tests
  • Git history is clean
  • If PR impacts documentation, docs team has been notified or an issue has been opened on the documentation repo

@ofek ofek changed the title [Prometheus base check] set request stream option to false set request stream option to false May 24, 2018
@masci masci changed the title set request stream option to false Set requests stream option to false when scraping Prometheus endpoints May 30, 2018
Copy link
Contributor

@masci masci left a comment

Choose a reason for hiding this comment

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

One small nit but feel free to ignore and merge

@@ -474,7 +474,7 @@ def poll(self, endpoint, pFormat=PrometheusFormat.PROTOBUF, headers=None):
disable_warnings(InsecureRequestWarning)
verify = False
try:
response = requests.get(endpoint, headers=headers, stream=True, timeout=10, cert=cert, verify=verify)
response = requests.get(endpoint, headers=headers, stream=False, timeout=10, cert=cert, verify=verify)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: the param already defaults to False so you can just remove stream altogether.

@masci masci merged commit b7fdff6 into master Jun 11, 2018
@masci masci deleted the antoine/prometheus-check-set-request-stream-option-to-false branch June 11, 2018 09:41
@antoinepouille
Copy link
Contributor Author

Thanks!

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

Successfully merging this pull request may close these issues.

3 participants