From 2a827d20e0a0dd130f4c7490cdca815131495542 Mon Sep 17 00:00:00 2001 From: Wilfred Tyler Gee Date: Mon, 22 Jan 2018 16:40:31 +1100 Subject: [PATCH 1/6] DB access consistency In preparation for #120, this is brining some consistency to the access of DB items so it will be easier to decouple from mongo if needed. --- bin/peas_shell | 30 ++++++++++++++---------------- pocs/core.py | 2 +- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/bin/peas_shell b/bin/peas_shell index 04f39741e..f29a51b2e 100755 --- a/bin/peas_shell +++ b/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 @@ -218,7 +215,8 @@ class PanSensorShell(cmd.Cmd): try: relay_info = relay_lookup[relay] - self.environment.serial_readers[relay_info['board']]['reader'].write("{},9".format(relay_info['pin'])) + serial_connection = self.environment.serial_readers[relay_info['board']]['reader'] + serial_connection.write("{},9".format(relay_info['pin'])) except Exception as e: print_warning("Problem toggling relay {}".format(relay)) print_warning(e) @@ -270,7 +268,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. diff --git a/pocs/core.py b/pocs/core.py index 0ac13c9d3..d6fc2837e 100644 --- a/pocs/core.py +++ b/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'] From da240316958dd24a4ab62b38d822fbf947d6bd51 Mon Sep 17 00:00:00 2001 From: Wilfred Tyler Gee Date: Mon, 22 Jan 2018 18:27:25 +1100 Subject: [PATCH 2/6] DB consistency * Make an `insert` method and call that from user code --- pocs/camera/canon_gphoto2.py | 3 +-- pocs/utils/database.py | 51 +++++++++++++++++++++++++++++------- 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/pocs/camera/canon_gphoto2.py b/pocs/camera/canon_gphoto2.py index fc1fd6725..e2f0812f4 100644 --- a/pocs/camera/canon_gphoto2.py +++ b/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, }) diff --git a/pocs/utils/database.py b/pocs/utils/database.py index 5b5ef817f..f8c1b6066 100644 --- a/pocs/utils/database.py +++ b/pocs/utils/database.py @@ -78,16 +78,18 @@ def __init__(self, db='panoptes', host='localhost', port=27017, connect=False): setattr(self, collection, getattr(db_handle, collection)) 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") @@ -101,12 +103,43 @@ 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 + _id = self.insert(collection, current_obj) + except AttributeError: + warn("Collection does not exist in db: {}".format(collection)) + except Exception as e: + warn("Problem inserting object into collection: {}".format(e)) + + return _id + + def insert(self, collection, obj): + """Insert an object into the collection provided. + + 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, warn("Collection not available") + + _id = None + try: + if 'type' not in obj: + 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 AttributeError: warn("Collection does not exist in db: {}".format(collection)) except Exception as e: From 911d3244050eca7359f69d7736cb8e0bc00704ba Mon Sep 17 00:00:00 2001 From: Wilfred Tyler Gee Date: Mon, 22 Jan 2018 18:29:03 +1100 Subject: [PATCH 3/6] Reverting line back (change already in #399) --- bin/peas_shell | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bin/peas_shell b/bin/peas_shell index f29a51b2e..5a5f1a5ae 100755 --- a/bin/peas_shell +++ b/bin/peas_shell @@ -215,8 +215,7 @@ class PanSensorShell(cmd.Cmd): try: relay_info = relay_lookup[relay] - serial_connection = self.environment.serial_readers[relay_info['board']]['reader'] - serial_connection.write("{},9".format(relay_info['pin'])) + self.environment.serial_readers[relay_info['board']]['reader'].write("{},9".format(relay_info['pin'])) except Exception as e: print_warning("Problem toggling relay {}".format(relay)) print_warning(e) From 42b94d628e25ce8d83b70ee1aeedc74f933555df Mon Sep 17 00:00:00 2001 From: Wilfred Tyler Gee Date: Mon, 22 Jan 2018 18:34:53 +1100 Subject: [PATCH 4/6] Handle metadata wrapping of `insert` better --- pocs/utils/database.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pocs/utils/database.py b/pocs/utils/database.py index f8c1b6066..1d425da7f 100644 --- a/pocs/utils/database.py +++ b/pocs/utils/database.py @@ -130,7 +130,12 @@ def insert(self, collection, obj): _id = None try: - if 'type' not in obj: + # If `data` key is present we assume it has "metadata" for object + if 'data' in obj: + # But still check for a `type` + if 'type' not in obj: + obj['type'] = collection + else: obj = { 'type': collection, 'data': obj, From 71cadb5e5f522e421ad5bd79fedf4cc8d68b6604 Mon Sep 17 00:00:00 2001 From: Wilfred Tyler Gee Date: Mon, 22 Jan 2018 23:11:04 +1100 Subject: [PATCH 5/6] Removing redundant checks and adding some tests --- pocs/tests/test_database.py | 21 +++++++++++++++++++++ pocs/utils/database.py | 4 ---- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/pocs/tests/test_database.py b/pocs/tests/test_database.py index 37941fd7b..339c81780 100644 --- a/pocs/tests/test_database.py +++ b/pocs/tests/test_database.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}) diff --git a/pocs/utils/database.py b/pocs/utils/database.py index 1d425da7f..c2afbd193 100644 --- a/pocs/utils/database.py +++ b/pocs/utils/database.py @@ -109,8 +109,6 @@ def insert_current(self, collection, obj, include_collection=True): if include_collection: _id = self.insert(collection, current_obj) - except AttributeError: - warn("Collection does not exist in db: {}".format(collection)) except Exception as e: warn("Problem inserting object into collection: {}".format(e)) @@ -145,8 +143,6 @@ def insert(self, collection, obj): # Insert record into db col = getattr(self, collection) _id = col.insert_one(obj).inserted_id - except AttributeError: - warn("Collection does not exist in db: {}".format(collection)) except Exception as e: warn("Problem inserting object into collection: {}".format(e)) From 8c897bfefda8acee815259dc532536b107a89bfc Mon Sep 17 00:00:00 2001 From: Wilfred Tyler Gee Date: Tue, 23 Jan 2018 11:41:35 +1100 Subject: [PATCH 6/6] PR updates * Add a logger to PanMongo * Some docstrings * Rename testing file to be mongo specific --- pocs/base.py | 2 +- .../{test_database.py => test_panmongo.py} | 0 pocs/utils/database.py | 35 +++++++++++++------ 3 files changed, 26 insertions(+), 11 deletions(-) rename pocs/tests/{test_database.py => test_panmongo.py} (100%) diff --git a/pocs/base.py b/pocs/base.py index 40de909dc..4d9d5b92b 100644 --- a/pocs/base.py +++ b/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 diff --git a/pocs/tests/test_database.py b/pocs/tests/test_panmongo.py similarity index 100% rename from pocs/tests/test_database.py rename to pocs/tests/test_panmongo.py diff --git a/pocs/utils/database.py b/pocs/utils/database.py index c2afbd193..1a63ae63e 100644 --- a/pocs/utils/database.py +++ b/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,6 +81,12 @@ 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. @@ -92,7 +102,7 @@ def insert_current(self, collection, obj, include_collection=True): 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: @@ -110,13 +120,18 @@ def insert_current(self, collection, obj, include_collection=True): if include_collection: _id = self.insert(collection, current_obj) except Exception as e: - warn("Problem inserting object into collection: {}".format(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. @@ -124,12 +139,12 @@ def insert(self, collection, obj): Returns: str: Mongo object ID of record in `collection`. """ - assert collection in self.collections, warn("Collection not available") + assert collection in self.collections, self._warn("Collection not available") _id = None try: - # If `data` key is present we assume it has "metadata" for object - if 'data' in obj: + # 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 @@ -144,7 +159,7 @@ def insert(self, collection, obj): 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