-
Notifications
You must be signed in to change notification settings - Fork 4
Description
In populse_db.storage_api.StorageFileAPI.primary_key(), we have:
dbs = self._get_database_session(connection_id, write=True)
The code is poorly documented, so I may misunderstand it, but it seems that write=True
is usually reserved for writing operations in a database. However, in this case, it is just a request to retrieve the primary key of a collection (perhaps this is where I am mistaken?).
So, if we do something like:
with self.project.database.data() as database_data:
foo = database_data.get_primary_key_name(COLLECTION_CURRENT)
we encounter the following error:
File "/data/Git_Projects/populse_db/python/populse_db/storage.py", line 290, in primary_key
return self._storage_api.primary_key(self._connection_id, self._path)
File "/data/Git_Projects/populse_db/python/populse_db/storage_api.py", line 369, in primary_key
dbs = self._get_database_session(connection_id, write=True)
File "/data/Git_Projects/populse_db/python/populse_db/storage_api.py", line 176, in _get_database_session
raise PermissionError(self._read_only_error)
PermissionError: database is read-only
To fix this, we can use:
with self.project.database.data(write=True) as database_data:
foo = database_data.get_primary_key_name(COLLECTION_CURRENT)
However, this seems strange for a simple database query.
The get_primary_key_name
method belongs to populse_mia.data_manager.database_mia.DatabaseMiaData. It uses:
`self.storage_data[collection_name].primary_key()[0]
Another possible fix would be to replace it with:
next(iter(self.storage_data[collection_name].keys()))
This seems to work, but it would be better to use a dedicated method from populse_db, such as populse_db.storage_api.StorageFileAPI.primary_key().
Additionally, are we sure that the primary key is always the first element in the dbs[collection].keys()
dictionary?
So, I have a few ways to fix this issue, but none seem entirely clean.
Ideally, the following should work:
with self.project.database.data() as database_data:
foo = database_data.get_primary_key_name(COLLECTION_CURRENT)
It also seems that write=True
is used very frequently (too frequently?) in populse_db.storage_api.StorageFileAPI
.
For instance, could we consider modifying database session access in StorageFileAPI to omit write=True
when we only need to make a read request and do not require indexing?
For example, in populse_db.storage_api.StorageFileAPI.primary_key()
, by using:
dbs = self._get_database_session(connection_id, write=False)
instead?