-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Support Python 3 for datadog_checks_base #1957
Conversation
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.
Really nice! I added two comments that are minor. This is great.
@@ -255,10 +259,10 @@ def get_metric_value_by_labels(messages, _metric, _m, metric_suffix): | |||
:return: value of the metric_name matched by the labels | |||
""" | |||
metric_name = '{}_{}'.format(_m, metric_suffix) | |||
expected_labels = set([(k, v) for k, v in _metric["labels"].iteritems() | |||
expected_labels = set([(k, v) for k, v in iteritems(_metric["labels"]) |
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.
Should we use get
instead?
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.
Yes, but to reduce the scope and ease review of this PR I chose to just focus on version incompatibilities.
@@ -40,7 +41,9 @@ def __init__(self, class_name, counter_name, log, instance_name = None, machine_ | |||
self._class_name = class_name | |||
else: | |||
if len(class_name_index_list) > 1: | |||
self.logger.warning("Class %s had multiple (%d) indices, using first" % (class_name, len(class_name_index_list))) | |||
self.logger.warning( | |||
"Class %s had multiple (%d) indices, using first" % (class_name, len(class_name_index_list)) |
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.
format
instead of %
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.
Same as above
ok fair enough |
Motivation
https://pythonclock.org
CI
https://travis-ci.org/DataDog/integrations-core/builds/409447772#L865
https://ci.appveyor.com/project/Datadog/integrations-core/build/master.6768#L761