Skip to content

Commit

Permalink
🔧 MAINTAIN: Remove BackendEntity.from_dbmodel and `BackendCollectio…
Browse files Browse the repository at this point in the history
…n.from_dbmodel` (#5359)

These methods are implementation details of the `PsqlDosBackend`,
and not accessed by the "front-end" ORM,
thus should not be required by other backend implementations.
  • Loading branch information
chrisjsewell authored Feb 17, 2022
1 parent 092919d commit da5f9b0
Show file tree
Hide file tree
Showing 11 changed files with 39 additions and 63 deletions.
14 changes: 0 additions & 14 deletions aiida/orm/implementation/entities.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,6 @@ def is_stored(self) -> bool:
:return: True if stored, False otherwise
"""

@classmethod
@abc.abstractmethod
def from_dbmodel(cls, dbmodel: Any, backend: 'Backend') -> EntityType:
"""Create this entity from a dbmodel."""


class BackendCollection(Generic[EntityType]):
"""Container class that represents a collection of entries of a particular backend entity."""
Expand All @@ -86,15 +81,6 @@ def __init__(self, backend: 'Backend'):
assert issubclass(self.ENTITY_CLASS, BackendEntity), 'Must set the ENTRY_CLASS class variable to an entity type'
self._backend = backend

def from_dbmodel(self, dbmodel):
"""
Create an entity from the backend dbmodel
:param dbmodel: the dbmodel to create the entity from
:return: the entity instance
"""
return self.ENTITY_CLASS.from_dbmodel(dbmodel, self.backend)

@property
def backend(self) -> 'Backend':
"""Return the backend."""
Expand Down
10 changes: 6 additions & 4 deletions aiida/orm/implementation/sqlalchemy/authinfos.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ def __init__(self, backend, computer, user):
super().__init__(backend)
type_check(user, users.SqlaUser)
type_check(computer, computers.SqlaComputer)
self._model = utils.ModelWrapper(DbAuthInfo(dbcomputer=computer.bare_model, aiidauser=user.bare_model), backend)
self._model = utils.ModelWrapper(
self.MODEL_CLASS(dbcomputer=computer.bare_model, aiidauser=user.bare_model), backend
)

@property
def id(self): # pylint: disable=invalid-name
Expand Down Expand Up @@ -68,15 +70,15 @@ def computer(self):
:return: :class:`aiida.orm.implementation.computers.BackendComputer`
"""
return self.backend.computers.from_dbmodel(self.model.dbcomputer)
return self.backend.computers.ENTITY_CLASS.from_dbmodel(self.model.dbcomputer, self.backend)

@property
def user(self):
"""Return the user associated with this instance.
:return: :class:`aiida.orm.implementation.users.BackendUser`
"""
return self._backend.users.from_dbmodel(self.model.aiidauser)
return self.backend.users.ENTITY_CLASS.from_dbmodel(self.model.aiidauser, self.backend)

def get_auth_params(self):
"""Return the dictionary of authentication parameters
Expand Down Expand Up @@ -123,7 +125,7 @@ def delete(self, pk):
session = self.backend.get_session()

try:
row = session.query(DbAuthInfo).filter_by(id=pk).one()
row = session.query(self.ENTITY_CLASS.MODEL_CLASS).filter_by(id=pk).one()
session.delete(row)
session.commit()
except NoResultFound:
Expand Down
16 changes: 7 additions & 9 deletions aiida/orm/implementation/sqlalchemy/comments.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,13 @@ class SqlaComment(entities.SqlaModelEntity[models.DbComment], BackendComment):

# pylint: disable=too-many-arguments
def __init__(self, backend, node, user, content=None, ctime=None, mtime=None):
"""
Construct a SqlaComment.
"""Construct a SqlaComment.
:param node: a Node instance
:param user: a User instance
:param content: the comment content
:param ctime: The creation time as datetime object
:param mtime: The modification time as datetime object
:return: a Comment object associated to the given node and user
"""
super().__init__(backend)
lang.type_check(user, users.SqlaUser) # pylint: disable=no-member
Expand All @@ -55,7 +53,7 @@ def __init__(self, backend, node, user, content=None, ctime=None, mtime=None):
lang.type_check(mtime, datetime, f'the given mtime is of type {type(mtime)}')
arguments['mtime'] = mtime

self._model = utils.ModelWrapper(models.DbComment(**arguments), backend)
self._model = utils.ModelWrapper(self.MODEL_CLASS(**arguments), backend)

def store(self):
"""Can only store if both the node and user are stored as well."""
Expand All @@ -82,11 +80,11 @@ def set_mtime(self, value):

@property
def node(self):
return self.backend.nodes.from_dbmodel(self.bare_model.dbnode)
return self.backend.nodes.ENTITY_CLASS.from_dbmodel(self.model.dbnode, self.backend)

@property
def user(self):
return self.backend.users.from_dbmodel(self.bare_model.user)
return self.backend.users.ENTITY_CLASS.from_dbmodel(self.model.user, self.backend)

def set_user(self, value):
self.model.user = value
Expand All @@ -113,7 +111,7 @@ def create(self, node, user, content=None, **kwargs):
:param content: the comment content
:return: a Comment object associated to the given node and user
"""
return SqlaComment(self.backend, node, user, content, **kwargs) # pylint: disable=abstract-class-instantiated
return self.ENTITY_CLASS(self.backend, node, user, content, **kwargs)

def delete(self, comment_id):
"""
Expand All @@ -131,7 +129,7 @@ def delete(self, comment_id):
session = self.backend.get_session()

try:
row = session.query(models.DbComment).filter_by(id=comment_id).one()
row = session.query(self.ENTITY_CLASS.MODEL_CLASS).filter_by(id=comment_id).one()
session.delete(row)
session.commit()
except NoResultFound:
Expand All @@ -147,7 +145,7 @@ def delete_all(self):
session = self.backend.get_session()

try:
session.query(models.DbComment).delete()
session.query(self.ENTITY_CLASS.MODEL_CLASS).delete()
session.commit()
except Exception as exc:
session.rollback()
Expand Down
8 changes: 4 additions & 4 deletions aiida/orm/implementation/sqlalchemy/computers.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class SqlaComputer(entities.SqlaModelEntity[DbComputer], BackendComputer):

def __init__(self, backend, **kwargs):
super().__init__(backend)
self._model = utils.ModelWrapper(DbComputer(**kwargs), backend)
self._model = utils.ModelWrapper(self.MODEL_CLASS(**kwargs), backend)

@property
def uuid(self):
Expand Down Expand Up @@ -60,7 +60,7 @@ def copy(self):
make_transient(dbcomputer)
session.add(dbcomputer)

newobject = self.__class__.from_dbmodel(dbcomputer) # pylint: disable=no-value-for-parameter
newobject = self.__class__.from_dbmodel(dbcomputer, self.backend)

return newobject

Expand Down Expand Up @@ -120,12 +120,12 @@ class SqlaComputerCollection(BackendComputerCollection):

def list_names(self):
session = self.backend.get_session()
return session.query(DbComputer.label).all()
return session.query(self.ENTITY_CLASS.MODEL_CLASS.label).all()

def delete(self, pk):
try:
session = self.backend.get_session()
row = session.get(DbComputer, pk)
row = session.get(self.ENTITY_CLASS.MODEL_CLASS, pk)
session.delete(row)
session.commit()
except SQLAlchemyError as exc:
Expand Down
6 changes: 1 addition & 5 deletions aiida/orm/implementation/sqlalchemy/convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,7 @@
"""
Module to get the backend instance from the Models instance
"""

try: # Python3
from functools import singledispatch
except ImportError: # Python2
from singledispatch import singledispatch
from functools import singledispatch

from aiida.backends.sqlalchemy.models.authinfo import DbAuthInfo
from aiida.backends.sqlalchemy.models.comment import DbComment
Expand Down
3 changes: 1 addition & 2 deletions aiida/orm/implementation/sqlalchemy/entities.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ def _class_check(cls):

@classmethod
def from_dbmodel(cls, dbmodel, backend):
"""
Create an AiiDA Entity from the corresponding ORM model and storage backend
"""Create an AiiDA Entity from the corresponding SQLA ORM model and storage backend
:param dbmodel: the SQLAlchemy model to create the entity from
:param backend: the corresponding storage backend
Expand Down
6 changes: 3 additions & 3 deletions aiida/orm/implementation/sqlalchemy/groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def __init__(self, backend, label, user, description='', type_string=''):
type_check(user, users.SqlaUser)
super().__init__(backend)

dbgroup = DbGroup(label=label, description=description, user=user.bare_model, type_string=type_string)
dbgroup = self.MODEL_CLASS(label=label, description=description, user=user.bare_model, type_string=type_string)
self._model = utils.ModelWrapper(dbgroup, backend)

@property
Expand Down Expand Up @@ -87,7 +87,7 @@ def type_string(self):

@property
def user(self):
return self._backend.users.from_dbmodel(self.model.user)
return self.backend.users.ENTITY_CLASS.from_dbmodel(self.model.user, self.backend)

@user.setter
def user(self, new_user):
Expand Down Expand Up @@ -283,6 +283,6 @@ class SqlaGroupCollection(BackendGroupCollection):
def delete(self, id): # pylint: disable=redefined-builtin
session = self.backend.get_session()

row = session.get(DbGroup, id)
row = session.get(self.ENTITY_CLASS.MODEL_CLASS, id)
session.delete(row)
session.commit()
6 changes: 3 additions & 3 deletions aiida/orm/implementation/sqlalchemy/logs.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def __init__(self, backend, time, loggername, levelname, dbnode_id, message='',
# pylint: disable=too-many-arguments
super().__init__(backend)
self._model = utils.ModelWrapper(
models.DbLog(
self.MODEL_CLASS(
time=time,
loggername=loggername,
levelname=levelname,
Expand Down Expand Up @@ -109,7 +109,7 @@ def delete(self, log_id):
session = self.backend.get_session()

try:
row = session.query(models.DbLog).filter_by(id=log_id).one()
row = session.query(self.ENTITY_CLASS.MODEL_CLASS).filter_by(id=log_id).one()
session.delete(row)
session.commit()
except NoResultFound:
Expand All @@ -125,7 +125,7 @@ def delete_all(self):
session = self.backend.get_session()

try:
session.query(models.DbLog).delete()
session.query(self.ENTITY_CLASS.MODEL_CLASS).delete()
session.commit()
except Exception as exc:
session.rollback()
Expand Down
18 changes: 10 additions & 8 deletions aiida/orm/implementation/sqlalchemy/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"""SqlAlchemy implementation of the `BackendNode` and `BackendNodeCollection` classes."""
# pylint: disable=no-name-in-module,import-error
from datetime import datetime
from typing import Any, Dict, Iterable, Tuple
from typing import Any, Dict, Iterable, Tuple, Type

from sqlalchemy.exc import SQLAlchemyError
from sqlalchemy.orm.exc import NoResultFound
Expand Down Expand Up @@ -83,7 +83,7 @@ def __init__(
type_check(mtime, datetime, f'the given mtime is of type {type(mtime)}')
arguments['mtime'] = mtime

self._model = sqla_utils.ModelWrapper(models.DbNode(**arguments), backend)
self._model = sqla_utils.ModelWrapper(self.MODEL_CLASS(**arguments), backend)

def clone(self):
"""Return an unstored clone of ourselves.
Expand All @@ -103,7 +103,7 @@ def clone(self):

clone = self.__class__.__new__(self.__class__) # pylint: disable=no-value-for-parameter
clone.__init__(self.backend, self.node_type, self.user)
clone._model = sqla_utils.ModelWrapper(models.DbNode(**arguments), self.backend) # pylint: disable=protected-access
clone._model = sqla_utils.ModelWrapper(self.MODEL_CLASS(**arguments), self.backend) # pylint: disable=protected-access
return clone

@property
Expand Down Expand Up @@ -157,7 +157,7 @@ def repository_metadata(self, value):
@property
def computer(self):
try:
return self.backend.computers.from_dbmodel(self.model.dbcomputer)
return self.backend.computers.ENTITY_CLASS.from_dbmodel(self.model.dbcomputer, self.backend)
except TypeError:
return None

Expand All @@ -172,7 +172,7 @@ def computer(self, computer):

@property
def user(self):
return self.backend.users.from_dbmodel(self.model.user)
return self.backend.users.ENTITY_CLASS.from_dbmodel(self.model.user, self.backend)

@user.setter
def user(self, user):
Expand Down Expand Up @@ -308,21 +308,23 @@ def attributes_keys(self) -> Iterable[str]:
class SqlaNodeCollection(BackendNodeCollection):
"""The collection of Node entries."""

ENTITY_CLASS = SqlaNode
ENTITY_CLASS: Type[SqlaNode] = SqlaNode

def get(self, pk):
session = self.backend.get_session()

try:
return self.ENTITY_CLASS.from_dbmodel(session.query(models.DbNode).filter_by(id=pk).one(), self.backend)
return self.ENTITY_CLASS.from_dbmodel(
session.query(self.ENTITY_CLASS.MODEL_CLASS).filter_by(id=pk).one(), self.backend
)
except NoResultFound:
raise exceptions.NotExistent(f"Node with pk '{pk}' not found") from NoResultFound

def delete(self, pk):
session = self.backend.get_session()

try:
row = session.query(models.DbNode).filter_by(id=pk).one()
row = session.query(self.ENTITY_CLASS.MODEL_CLASS).filter_by(id=pk).one()
session.delete(row)
session.commit()
except NoResultFound:
Expand Down
12 changes: 3 additions & 9 deletions aiida/orm/implementation/sqlalchemy/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def __init__(self, backend, email, first_name, last_name, institution):
# pylint: disable=too-many-arguments
super().__init__(backend)
self._model = utils.ModelWrapper(
DbUser(email=email, first_name=first_name, last_name=last_name, institution=institution), backend
self.MODEL_CLASS(email=email, first_name=first_name, last_name=last_name, institution=institution), backend
)

@property
Expand Down Expand Up @@ -67,11 +67,5 @@ class SqlaUserCollection(BackendUserCollection):
ENTITY_CLASS = SqlaUser

def create(self, email, first_name='', last_name='', institution=''): # pylint: disable=arguments-differ
"""
Create a user with the provided email address
:return: A new user object
:rtype: :class:`aiida.orm.User`
"""
# pylint: disable=abstract-class-instantiated
return SqlaUser(self.backend, email, first_name, last_name, institution)
""" Create a user with the provided email address"""
return self.ENTITY_CLASS(self.backend, email, first_name, last_name, institution)
3 changes: 1 addition & 2 deletions tests/orm/implementation/test_groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ def test_creation_from_dbgroup(backend):
group.store()
group.add_nodes([node.backend_entity])

dbgroup = group.bare_model
gcopy = backend.groups.from_dbmodel(dbgroup)
gcopy = group.__class__.from_dbmodel(group.bare_model, backend)

assert group.pk == gcopy.pk
assert group.uuid == gcopy.uuid
Expand Down

2 comments on commit da5f9b0

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'pytest-benchmarks:ubuntu-18.04,psql_dos'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: da5f9b0 Previous: 092919d Ratio
tests/benchmark/test_archive.py::test_export[no-objects] 1.3251595211847482 iter/sec (stddev: 0.12486) 2.6956863217734854 iter/sec (stddev: 0.061935) 2.03
tests/benchmark/test_archive.py::test_export[with-objects] 1.2919071265454785 iter/sec (stddev: 0.25610) 2.7170246918254866 iter/sec (stddev: 0.057016) 2.10

This comment was automatically generated by workflow using github-action-benchmark.

CC: @chrisjsewell @giovannipizzi

@chrisjsewell
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, https://aiidateam.github.io/aiida-core/dev/bench/ubuntu-18.04/psql_dos/
Don't see any reason why this would be the case, but will keep an eye on it

Please sign in to comment.