nonaggregated column 'mike.features_metrics.value' - fix MySQL 8 error

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**.
+

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
@@ -175,15 +182,16 @@ 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
             }
         )
 
         for row in iter(cursor):
-            yield dict(zip(
-                ('date', 'metric', 'value'),
-                row
-            ))
+            yield {
+                'date': str(row[0]),
+                'metric': row[1],
+                'value': float(row[2]),
+            }

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;

test/test_storage.py

@@ -24,19 +24,31 @@ def __init__(self):
         }
 
 
+TIMESTAMP = '2019-03-02 20:22:24'
+TIMESTAMP_LATER = '2019-03-04 10: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')
+
     # 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)
+
+    # 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()
+    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'
@@ -54,3 +66,21 @@ 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_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}
+    ]