From a7b06263c182f4e6421bd2d5559c7db9e854593c Mon Sep 17 00:00:00 2001 From: macbre Date: Sat, 2 Mar 2019 20:43:23 +0100 Subject: [PATCH 1/2] test_storage: improve tests --- README.md | 3 +++ mycroft_holmes/storage.py | 15 +++++++++++---- schema.sql | 2 +- test/test_storage.py | 17 +++++++++++++++-- 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index dba1285..98e92c7 100644 --- a/README.md +++ b/README.md @@ -171,3 +171,6 @@ $ make test # access mysql console with the above credentials $ make mysql_cli ``` + + > Please note that **tests truncate the tables**. + \ No newline at end of file diff --git a/mycroft_holmes/storage.py b/mycroft_holmes/storage.py index dd15618..ca567ff 100644 --- a/mycroft_holmes/storage.py +++ b/mycroft_holmes/storage.py @@ -109,17 +109,20 @@ def push(self, feature_id, feature_metrics): self.data[feature_id] = feature_metrics - def commit(self): + def commit(self, timestamp=None): """ Store metrics collected via push() + + :type timestamp str """ try: cursor = self.storage.cursor() self.storage.start_transaction() # take the current timestamp and use it to make this value consistent - cursor.execute('SELECT /* mycroft_holmes */ NOW()') - timestamp = cursor.fetchone()[0] + if timestamp is None: + cursor.execute('SELECT /* mycroft_holmes */ NOW()') + timestamp = cursor.fetchone()[0] self.logger.info("Using timestamp %s", timestamp) @@ -160,7 +163,11 @@ def get_the_latest_timestamp(self): cursor = self.storage.cursor() cursor.execute("SELECT /* mycroft_holmes */ MAX(timestamp) FROM features_metrics") - return cursor.fetchone()[0] + value = cursor.fetchone()[0] + + # cast datetime.datetime to string + return str(value) if value else None + except MySqlError as ex: self.logger.error('Storage error occured: %s', ex) return None diff --git a/schema.sql b/schema.sql index 464be16..8045e45 100644 --- a/schema.sql +++ b/schema.sql @@ -5,7 +5,7 @@ CREATE TABLE `features_metrics` ( `feature` varchar(32) NOT NULL, `metric` varchar(32) NOT NULL, `value` int(9) NOT NULL, - `timestamp` datetime NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, + `timestamp` datetime NOT NULL, PRIMARY KEY (`entry_id`), KEY `feature_metric_timestamp_idx` (`feature`,`metric`,`timestamp`) ) CHARSET=utf8; diff --git a/test/test_storage.py b/test/test_storage.py index 5d28bdc..ce25a4b 100644 --- a/test/test_storage.py +++ b/test/test_storage.py @@ -24,19 +24,29 @@ def __init__(self): } +TIMESTAMP = '2019-03-02 20:22:24' + + def test_storage(): if environ.get('TEST_DATABASE') is None: raise SkipTest('TEST_DATABASE env variable needs to be set to run this test.') storage = MetricsStorage(config=ConfigForMetricsStorage(), use_slave=False) + # clean up the storage + cursor = storage.storage.cursor() + cursor.execute('TRUNCATE TABLE features_metrics') + + # no data so far + # assert storage.get_the_latest_timestamp() is None + # push some metrics and later on try to get them storage.push('foo', {'score': 123, 'bar/metric': 42.458}) storage.push('bar', {'score': 1, 'bar/metric': -3}) - storage.commit() + storage.commit(timestamp=TIMESTAMP) storage.push('bar', {'score': 5, 'bar/metric': -4}) - storage.commit() + storage.commit(timestamp=TIMESTAMP) assert storage.get(feature_id='foo', feature_metric='score') == 123 assert storage.get(feature_id='foo', feature_metric='bar/metric') == 42.46, 'Storage keeps floats with scale of 2' @@ -54,3 +64,6 @@ def test_storage(): metric = Metric(feature_name='Foo', config=ConfigForMetricsStorage(), spec={'name': 'bar/metric'}) assert metric.value == 42.46, 'Get the most recent value from the storage' assert isinstance(metric.value, float), 'The value is returned as a float' + + # check the latest timestamp + assert storage.get_the_latest_timestamp() == TIMESTAMP From 4c55d9a884126ca497ddfc2c9d9511b8c752ed08 Mon Sep 17 00:00:00 2001 From: macbre Date: Sat, 2 Mar 2019 22:51:35 +0100 Subject: [PATCH 2/2] Storage| get_feature_metrics_history - nonaggregated column 'mike.features_metrics.value' Fixes #75 --- mycroft_holmes/storage.py | 11 ++++++----- test/test_storage.py | 27 ++++++++++++++++++++++----- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/mycroft_holmes/storage.py b/mycroft_holmes/storage.py index ca567ff..3bac4a2 100644 --- a/mycroft_holmes/storage.py +++ b/mycroft_holmes/storage.py @@ -182,7 +182,7 @@ def get_feature_metrics_history(self, feature__id): cursor = self.storage.cursor() cursor.execute( - "SELECT /* mycroft_holmes */ DATE(timestamp) AS date, metric, value " + "SELECT /* mycroft_holmes */ DATE(timestamp) AS date, metric, MAX(value) as value " "FROM features_metrics WHERE feature = %(feature)s GROUP BY date, metric", { 'feature': feature__id @@ -190,7 +190,8 @@ def get_feature_metrics_history(self, feature__id): ) for row in iter(cursor): - yield dict(zip( - ('date', 'metric', 'value'), - row - )) + yield { + 'date': str(row[0]), + 'metric': row[1], + 'value': float(row[2]), + } diff --git a/test/test_storage.py b/test/test_storage.py index ce25a4b..2293975 100644 --- a/test/test_storage.py +++ b/test/test_storage.py @@ -25,6 +25,7 @@ def __init__(self): TIMESTAMP = '2019-03-02 20:22:24' +TIMESTAMP_LATER = '2019-03-04 10:22:24' def test_storage(): @@ -37,17 +38,18 @@ def test_storage(): cursor = storage.storage.cursor() cursor.execute('TRUNCATE TABLE features_metrics') - # no data so far - # assert storage.get_the_latest_timestamp() is None - # push some metrics and later on try to get them storage.push('foo', {'score': 123, 'bar/metric': 42.458}) storage.push('bar', {'score': 1, 'bar/metric': -3}) storage.commit(timestamp=TIMESTAMP) - storage.push('bar', {'score': 5, 'bar/metric': -4}) + # multiple values in the same day - test values aggregation + storage.push('bar', {'score': 1, 'bar/metric': 6}) storage.commit(timestamp=TIMESTAMP) + storage.push('bar', {'score': 5, 'bar/metric': -4}) + storage.commit(timestamp=TIMESTAMP_LATER) + assert storage.get(feature_id='foo', feature_metric='score') == 123 assert storage.get(feature_id='foo', feature_metric='bar/metric') == 42.46, 'Storage keeps floats with scale of 2' @@ -66,4 +68,19 @@ def test_storage(): assert isinstance(metric.value, float), 'The value is returned as a float' # check the latest timestamp - assert storage.get_the_latest_timestamp() == TIMESTAMP + assert storage.get_the_latest_timestamp() == TIMESTAMP_LATER + + # get metrics history + assert list(storage.get_feature_metrics_history(feature__id='not_existing')) == [], 'Not existing feature' + + assert list(storage.get_feature_metrics_history(feature__id='foo')) == [ + {'date': '2019-03-02', 'metric': 'bar/metric', 'value': 42.46}, + {'date': '2019-03-02', 'metric': 'score', 'value': 123}, + ] + + assert list(storage.get_feature_metrics_history(feature__id='bar')) == [ + {'date': '2019-03-02', 'metric': 'bar/metric', 'value': 6.0}, # get max value for the day + {'date': '2019-03-02', 'metric': 'score', 'value': 1.0}, + {'date': '2019-03-04', 'metric': 'bar/metric', 'value': -4.0}, + {'date': '2019-03-04', 'metric': 'score', 'value': 5.0} + ]