Skip to content

Commit

Permalink
Dependencies: update sqlalchemy-utils dependency requirements (#4855)
Browse files Browse the repository at this point in the history
In `767f57a02b1ba40421ab9b17a67b2532aff556c6` the `sqlalchemy`
dependency was pinned to the `v1.3` minor version, because the recent
`v1.4.0` release would break `sqlalchemy-utils`. The latter has now
released `v0.37.0` which is compatible with `sqlalchemy==1.4`. In theory
this would allows us to support `v1.4` as well, except our own code
breaks with this version since we access quite a few protected
attributes in the query builder implementation.

Since `sqlalchemy-utils` does not follow semver, we have to pin it to
the `0.37.x` series for now. Since version `0.37.0` and `0.37.1` were
yanked as they did not specify the `python_requires` keyword, we set the
lower boundary to `0.37.2`.

The constructor of the `DbLog` class was simplified. Its only purpose is
to make sure that the `_metadata` column is properly initialized. The
column has an underscore prepended because otherwise it would clash with
an attribute of the SqlAlchemy base class, however, the constructor
takes just `metadata` so this has to be transferred. In addition, it
should be initialized to an empty dict if it is `None`. All other
initialization code that was in the constructor will be taken care of by
simply calling the parent class' constructor.

Finally, the `force_instant_defaults` listener of `sqlalchemy-utils` was
updated in v0.37.5 which caused certain tests to fail. The fix ensured
that the `kwargs` passed into the constructor of a model instance would
be updated with the defaults of the column definitions that are set on
the instance by the listener. The problem is that some of our models,
e.g. `DbComment`, rely no distinguishing between the column value being
explicitly passed by the caller or through default. Most notably this is
for the `mtime` which our implementation wants to have undefined before
storing, unless explicitly passed in the constructor. Due to the change
in `sqlalchemy-utils` this distinction could no longer be made and so
the only solution was to copy the original implementation of the
listener utility.
  • Loading branch information
sphuber authored Aug 2, 2021
1 parent 58c6d9f commit a468519
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 26 deletions.
25 changes: 21 additions & 4 deletions aiida/backends/sqlalchemy/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,30 @@
# For further information please visit http://www.aiida.net #
###########################################################################
"""Module to define the database models for the SqlAlchemy backend."""

from sqlalchemy_utils import force_instant_defaults
import sqlalchemy as sa

# SqlAlchemy does not set default values for table columns upon construction of a new instance, but will only do so
# when storing the instance. Any attributes that do not have a value but have a defined default, will be populated with
# this default. This does mean however, that before the instance is stored, these attributes are undefined, for example
# the UUID of a new instance. In Django this behavior is the opposite and more in intuitive because when one creates for
# example a `Node` instance in memory, it will already have a UUID. The following function call will force SqlAlchemy to
# behave the same as Django and set model attribute defaults upon instantiation.
force_instant_defaults()
# behave the same as Django and set model attribute defaults upon instantiation. Note that this functionality used to be
# provided by the ``sqlalchemy_utils.force_instant_defaults`` utility function. However, this function's behavior was
# changed in v0.37.5, where the ``sqlalchemy_utils.listeners.instant_defaults_listener`` was changed to update the
# original ``kwargs`` passed to the constructor, with the default values from the column definitions. This broke the
# constructor of certain of our database models, e.g. `DbComment`, which needs to distinguish between the value of the
# ``mtime`` column being defined by the caller as opposed to the default. This is why we revert this change by copying
# the old implementation of the listener.


def instant_defaults_listener(target, _, __):
"""Loop over the columns of the target model instance and populate defaults."""
for key, column in sa.inspect(target.__class__).columns.items():
if hasattr(column, 'default') and column.default is not None:
if callable(column.default.arg):
setattr(target, key, column.default.arg(target))
else:
setattr(target, key, column.default.arg)


sa.event.listen(sa.orm.mapper, 'init', instant_defaults_listener)
2 changes: 1 addition & 1 deletion aiida/backends/sqlalchemy/models/comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def __init__(self, *args, **kwargs):
"""Adding mtime attribute if not present."""
super().__init__(*args, **kwargs)
# The behavior of an unstored Comment instance should be that all its attributes should be initialized in
# accordance with the defaults specified on the collums, i.e. if a default is specified for the `uuid` column,
# accordance with the defaults specified on the columns, i.e. if a default is specified for the `uuid` column,
# then an unstored `DbComment` instance should have a default value for the `uuid` attribute. The exception here
# is the `mtime`, that we do not want to be set upon instantiation, but only upon storing. However, in
# SqlAlchemy a default *has* to be defined if one wants to get that value upon storing. But since defining a
Expand Down
21 changes: 5 additions & 16 deletions aiida/backends/sqlalchemy/models/log.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,10 @@ class DbLog(Base):

dbnode = relationship('DbNode', backref=backref('dblogs', passive_deletes='all', cascade='merge'))

def __init__(self, time, loggername, levelname, dbnode_id, **kwargs):
"""Setup initial value for the class attributes."""
if 'uuid' in kwargs:
self.uuid = kwargs['uuid']
if 'message' in kwargs:
self.message = kwargs['message']
if 'metadata' in kwargs:
self._metadata = kwargs['metadata'] or {}
else:
self._metadata = {}

self.time = time
self.loggername = loggername
self.levelname = levelname
self.dbnode_id = dbnode_id

def __str__(self):
return f'DbLog: {self.levelname} for node {self.dbnode.id}: {self.message}'

def __init__(self, *args, **kwargs):
"""Construct new instance making sure the `_metadata` column is initialized to empty dict if `None`."""
super().__init__(*args, **kwargs)
self._metadata = kwargs.pop('metadata', {}) or {}
2 changes: 1 addition & 1 deletion environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ dependencies:
- pyyaml~=5.1
- reentry~=1.3
- simplejson~=3.16
- sqlalchemy-utils~=0.36.0
- sqlalchemy-utils~=0.37.2
- sqlalchemy~=1.3.10
- tabulate~=0.8.5
- tqdm~=4.45
Expand Down
2 changes: 1 addition & 1 deletion requirements/requirements-py-3.7.txt
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ sphinxcontrib-serializinghtml==1.1.4
sphinxext-rediraffe==0.2.5
SQLAlchemy==1.3.23
sqlalchemy-diff==0.1.3
SQLAlchemy-Utils==0.36.8
SQLAlchemy-Utils==0.37.8
sqlparse==0.4.1
sympy==1.7.1
tabulate==0.8.7
Expand Down
2 changes: 1 addition & 1 deletion requirements/requirements-py-3.8.txt
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ sphinxcontrib-serializinghtml==1.1.4
sphinxext-rediraffe==0.2.5
SQLAlchemy==1.3.23
sqlalchemy-diff==0.1.3
SQLAlchemy-Utils==0.36.8
SQLAlchemy-Utils==0.37.8
sqlparse==0.4.1
sympy==1.7.1
tabulate==0.8.7
Expand Down
2 changes: 1 addition & 1 deletion requirements/requirements-py-3.9.txt
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ sphinxcontrib-serializinghtml==1.1.4
sphinxext-rediraffe==0.2.5
SQLAlchemy==1.3.23
sqlalchemy-diff==0.1.3
SQLAlchemy-Utils==0.36.8
SQLAlchemy-Utils==0.37.8
sqlparse==0.4.1
sympy==1.7.1
tabulate==0.8.7
Expand Down
2 changes: 1 addition & 1 deletion setup.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
"pyyaml~=5.1",
"reentry~=1.3",
"simplejson~=3.16",
"sqlalchemy-utils~=0.36.0",
"sqlalchemy-utils~=0.37.2",
"sqlalchemy~=1.3.10",
"tabulate~=0.8.5",
"tqdm~=4.45",
Expand Down

0 comments on commit a468519

Please sign in to comment.