Add handlers for logging to graphite #6

Merged
merged 12 commits into from Mar 14, 2014

Projects

None yet

2 participants

@miltontony
Member

No description provided.

@jerith jerith commented on an outdated diff Mar 14, 2014
shortener/tests/test_api.py
@inlineCallbacks
def test_create_url_no_user_token(self):
yield ShortenerTables(self.account, self.conn).create_tables()
+ self.service.metrics.carbon_client.startService()
+ yield self.service.metrics.carbon_client.connect_d
+
+ self.assertEqual(self.tr.value(), "")
@jerith
jerith Mar 14, 2014 Praekelt Consulting member

You have these three lines in a bunch of tests. Maybe they should be in a helper method or part of setUp()?

@jerith jerith commented on an outdated diff Mar 14, 2014
shortener/tests/test_api.py
@@ -139,10 +192,14 @@ def test_short_url_generation(self):
url4 = yield self.service.shorten_url(url + '4')
urls = [url1, url2, url3, url4]
self.assertEqual(len(set(urls)), 4)
+ conn_queue = self.tr.value().strip().split('\n')
+ self.assertEqual(len(conn_queue), 4)
@jerith
jerith Mar 14, 2014 Praekelt Consulting member

You can use .splitlines() instead of .strip().split('\n'). Also, it's probably worth adding a helper to extract the metrics.

@miltontony

@jerith shouldn't this be self.connect_d.callback(protocol) directly?

Member

@jerith nevermind, I think I see what you're doing there. Creating a temp variable of the existing connect_d and resetting it before adding the callback to the temp variable.

@miltontony
Member

👍

@jerith
Member
jerith commented Mar 14, 2014

👍

@miltontony miltontony merged commit d43d541 into develop Mar 14, 2014

1 check passed

Details default The Travis CI build passed
@miltontony miltontony deleted the feature/issue-6-push-metrics-to-graphite branch Mar 14, 2014
@miltontony miltontony restored the feature/issue-6-push-metrics-to-graphite branch Mar 22, 2014
@miltontony miltontony deleted the feature/issue-6-push-metrics-to-graphite branch Mar 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment