DB access consistency #408
Conversation
In preparation for panoptes#120, this is brining some consistency to the access of DB items so it will be easier to decouple from mongo if needed.
Codecov Report
@@ Coverage Diff @@
## develop #408 +/- ##
==========================================
+ Coverage 69.05% 69.15% +0.1%
==========================================
Files 60 60
Lines 4757 4773 +16
Branches 656 659 +3
==========================================
+ Hits 3285 3301 +16
+ Misses 1297 1295 -2
- Partials 175 177 +2
Continue to review full report at Codecov.
|
* Make an `insert` method and call that from user code
| @@ -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'), | |||
There was a problem hiding this comment.
Nothing changed except the wrapping, right?
If so, why was this changed, as it was 100 chars wide, which I thought we'd agreed on.
There was a problem hiding this comment.
100 char limit means it wraps at 99, which this line is. From the pep8:
For code maintained exclusively or primarily by a team that can reach agreement on this issue, it is okay to increase the nominal line length from 80 to 100 characters (effectively increasing the maximum length to 99 characters)
pocs/utils/database.py
Outdated
| @@ -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. | |||
There was a problem hiding this comment.
Thanks for adding the periods! 😉
pocs/utils/database.py
Outdated
| _id = None | ||
| try: | ||
| # If `data` key is present we assume it has "metadata" for object | ||
| if 'data' in obj: |
There was a problem hiding this comment.
if isinstance(obj, dict) and 'data' in obj:
pocs/utils/database.py
Outdated
|
|
||
| _id = None | ||
| try: | ||
| # If `data` key is present we assume it has "metadata" for object |
There was a problem hiding this comment.
What does this mean: it has "metadata" for object
What does it refer to there?
And what is the metadata?
pocs/utils/database.py
Outdated
|
|
||
| # 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)) |
There was a problem hiding this comment.
Could you add an additional message with the obj to be inserted?
And note that this is not written to the log file, which would really be useful!
There was a problem hiding this comment.
Fixed this so now we have a logger in PanMongo and warnings will go to appropriate place.
| rec = {'test': 'insert'} | ||
| db.insert('config', rec) | ||
|
|
||
| record = db.config.find({'data.test': {'$exists': 1}}) |
There was a problem hiding this comment.
This seems very mongo specific. Are you not attempting to address that ... yet?
There was a problem hiding this comment.
Correct. I was thinking of actually renaming to test_mongo.py, which could be part of this PR. Actually addressing anything non-mongo is for another PR.
There was a problem hiding this comment.
Renamed to test_panmongo.py for now.
| obj (dict or str): Object to be inserted. | ||
|
|
||
| Returns: | ||
| str: Mongo object ID of record in `collection`. |
There was a problem hiding this comment.
There currently isn't any error that is raised as we just catch the exceptions and give a warning. Not sure if that is ideal or not but it does allow operations to continue.
* Add a logger to PanMongo * Some docstrings * Rename testing file to be mongo specific
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.