DB access consistency

bin/peas_shell

@@ -15,7 +15,6 @@ from peas.webcam import Webcam
 
 from pocs.utils import current_time
 from pocs.utils.config import load_config
-from pocs.utils.logger import get_root_logger
 from pocs.utils.database import PanMongo
 
 
@@ -44,15 +43,19 @@ class PanSensorShell(cmd.Cmd):
     def do_status(self, *arg):
         """ Get the entire system status and print it pretty like! """
         if self._keep_looping:
-            console.color_print("{:>12s}: ".format('Loop Timer'), "default", "active", "lightgreen")
+            console.color_print("{:>12s}: ".format('Loop Timer'),
+                                "default", "active", "lightgreen")
         else:
-            console.color_print("{:>12s}: ".format('Loop Timer'), "default", "inactive", "yellow")
+            console.color_print("{:>12s}: ".format('Loop Timer'),
+                                "default", "inactive", "yellow")
 
         for sensor_name in ['environment', 'weather', 'webcams']:
             if sensor_name in self.active_sensors:
-                console.color_print("{:>12s}: ".format(sensor_name.title()), "default", "active", "lightgreen")
+                console.color_print("{:>12s}: ".format(sensor_name.title()),
+                                    "default", "active", "lightgreen")
             else:
-                console.color_print("{:>12s}: ".format(sensor_name.title()), "default", "inactive", "yellow")
+                console.color_print("{:>12s}: ".format(sensor_name.title()),
+                                    "default", "inactive", "yellow")
 
     def do_last_reading(self, device):
         """ Gets the last reading from the device. """
@@ -63,15 +66,9 @@ class PanSensorShell(cmd.Cmd):
             print_warning('No such sensor: {!r}'.format(device))
             return
 
-        rec = None
-        if device == 'weather':
-            rec = self.db.current.find_one({'type': 'weather'})
-        elif device == 'environment':
-            rec = self.db.current.find_one({'type': 'environment'})
-        else:
-            print_warning('No such sensor: {!r}'.format(device))
-            return
-        if not rec:
+        rec = self.db.get_current(device)
+
+        if rec is None:
             print_warning('No reading found for {!r}'.format(device))
             return
 
@@ -270,7 +267,7 @@ class PanSensorShell(cmd.Cmd):
 
     def do_change_delay(self, *arg):
         """Change the timing between reads from the named sensor."""
-        # NOTE: For at least the Arduinos, we should not need a delay and a timer, but 
+        # NOTE: For at least the Arduinos, we should not need a delay and a timer, but
         # simply a separate thread, reading from the board as data is available.
         # We might use a delay to deal with the case where the device is off-line
         # but we want to periodically check if it becomes available.

pocs/base.py

@@ -51,7 +51,7 @@ def __init__(self, *args, **kwargs):
 
         # Set up connection to database
         db = kwargs.get('db', self.config['db']['name'])
-        _db = PanMongo(db=db)
+        _db = PanMongo(db=db, logger=self.logger)
 
         self.db = _db
 

pocs/camera/canon_gphoto2.py

@@ -237,10 +237,9 @@ def process_exposure(self, info, signal_event, exposure_process=None):
             fits_utils.fpack(fits_path)
 
         self.logger.debug("Adding image metadata to db: {}".format(image_id))
-        self.db.observations.insert_one({
+        self.db.insert('observations', {
             'data': info,
             'date': current_time(datetime=True),
-            'type': 'observations',
             'sequence_id': seq_id,
         })
 

pocs/core.py

@@ -354,7 +354,7 @@ def is_weather_safe(self, stale=180):
             pass
 
         try:
-            record = self.db.current.find_one({'type': 'weather'})
+            record = self.db.get_current('weather')
 
             is_safe = record['data'].get('safe', False)
             timestamp = record['date']

pocs/tests/test_database.py → pocs/tests/test_panmongo.py

@@ -34,8 +34,29 @@ def test_insert_and_no_collection(db):
     db.current.remove({'type': 'config'})
 
 
+def test_simple_insert(db):
+    rec = {'test': 'insert'}
+    db.insert('config', rec)
+
+    record = db.config.find({'data.test': {'$exists': 1}})
+    assert record.count() == 1
+
+    db.current.remove({'type': 'config'})
+
+
 # Filter out (hide) "UserWarning: Collection not available"
 @pytest.mark.filterwarnings('ignore')
 def test_bad_collection(db):
     with pytest.raises(AssertionError):
         db.insert_current('foobar', {'test': 'insert'})
+
+    with pytest.raises(AssertionError):
+        db.insert('foobar', {'test': 'insert'})
+
+
+def test_bad_object(db):
+    with pytest.warns(UserWarning):
+        db.insert_current('observations', {'junk': db})
+
+    with pytest.warns(UserWarning):
+        db.insert('observations', {'junk': db})

pocs/utils/database.py

@@ -29,7 +29,7 @@ def get_shared_mongo_client(host, port, connect):
 
 class PanMongo(object):
 
-    def __init__(self, db='panoptes', host='localhost', port=27017, connect=False):
+    def __init__(self, db='panoptes', host='localhost', port=27017, connect=False, logger=None):
         """Connection to the running MongoDB instance
 
         This is a collection of parameters that are initialized when the unit
@@ -49,11 +49,15 @@ def __init__(self, db='panoptes', host='localhost', port=27017, connect=False):
 
         Args:
             db (str, optional): Name of the database containing the PANOPTES collections.
-            host (str, optional): hostname running MongoDB
-            port (int, optional): port running MongoDb
-            connect (bool, optional): Connect to mongo on create, defaults to True
+            host (str, optional): hostname running MongoDB.
+            port (int, optional): port running MongoDb.
+            connect (bool, optional): Connect to mongo on create, defaults to True.
+            logger (None, optional): An instance of the logger.
 
         """
+        if logger is not None:
+            self.logger = logger
+
         # Get the mongo client
         self._client = get_shared_mongo_client(host, port, connect)
 
@@ -77,20 +81,28 @@ def __init__(self, db='panoptes', host='localhost', port=27017, connect=False):
             # Add the collection as an attribute
             setattr(self, collection, getattr(db_handle, collection))
 
+    def _warn(self, *args, **kwargs):
+        if hasattr(self, 'logger'):
+            self.logger.warning(*args, **kwargs)
+        else:
+            warn(*args)
+
     def insert_current(self, collection, obj, include_collection=True):
-        """Insert an object into both the `current` collection and the collection provided
+        """Insert an object into both the `current` collection and the collection provided.
 
         Args:
-            collection (str): Name of valid collection within panoptes db
-            obj (dict or str): Object to be inserted
+            collection (str): Name of valid collection within panoptes db.
+            obj (dict or str): Object to be inserted.
             include_collection (bool): Whether to also update the collection,
-                defaults to True
+                defaults to True.
 
         Returns:
-            str: Mongo object ID of record in `collection`
+            str: Mongo object ID of record. If `include_collection` is True, will
+                be the id of the object in the `collection`, otherwise will be the
+                id of object in the `current` collection.
         """
         if include_collection:
-            assert collection in self.collections, warn("Collection not available")
+            assert collection in self.collections, self._warn("Collection not available")
 
         _id = None
         try:
@@ -101,16 +113,53 @@ def insert_current(self, collection, obj, include_collection=True):
             }
 
             # Update `current` record
-            self.current.replace_one({'type': collection}, current_obj, True)
+            _id = self.current.replace_one(
+                {'type': collection}, current_obj, True  # True for upsert
+            ).upserted_id
 
             if include_collection:
-                # Insert record into db
-                col = getattr(self, collection)
-                _id = col.insert_one(current_obj).inserted_id
-        except AttributeError:
-            warn("Collection does not exist in db: {}".format(collection))
+                _id = self.insert(collection, current_obj)
+        except Exception as e:
+            self._warn("Problem inserting object into collection: {}, {!r}".format(e, current_obj))
+
+        return _id
+
+    def insert(self, collection, obj):
+        """Insert an object into the collection provided.
+
+        The `obj` to be stored in a collection should include the `type`
+        and `date` metadata as well as a `data` key that contains the actual
+        object data. If these keys are not provided then `obj` will be wrapped
+        in a corresponding object that does contain the metadata.
+
+        Args:
+            collection (str): Name of valid collection within panoptes db.
+            obj (dict or str): Object to be inserted.
+
+        Returns:
+            str: Mongo object ID of record in `collection`.
+        """
+        assert collection in self.collections, self._warn("Collection not available")
+
+        _id = None
+        try:
+            # If `data` key is present we assume it has "metadata" (see above).
+            if isinstance(obj, dict) and 'data' in obj:
+                # But still check for a `type`
+                if 'type' not in obj:
+                    obj['type'] = collection
+            else:
+                obj = {
+                    'type': collection,
+                    'data': obj,
+                    'date': current_time(datetime=True),
+                }
+
+            # Insert record into db
+            col = getattr(self, collection)
+            _id = col.insert_one(obj).inserted_id
         except Exception as e:
-            warn("Problem inserting object into collection: {}".format(e))
+            self._warn("Problem inserting object into collection: {}, {!r}".format(e, obj))
 
         return _id