Skip to content

Commit

Permalink
Some cleanup of the code from remaining print messages, misplaced calls,
Browse files Browse the repository at this point in the history
leftover erroneous uses of the get_scoped_session() method for SQLA,
putting back in place some dependencies that I had removed, etc.
  • Loading branch information
giovannipizzi committed Mar 21, 2017
1 parent 7f6e80e commit 32cc238
Show file tree
Hide file tree
Showing 18 changed files with 234 additions and 166 deletions.
1 change: 0 additions & 1 deletion .travis-data/test_daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ def print_daemon_log():

def have_finished(pks):
finished_list = [load_node(pk).has_finished() for pk in pks]
print {pk: (load_node(pk).get_state(), load_node(pk).has_finished()) for pk in pks}
num_finished = len([_ for _ in finished_list if _])
print "{}/{} finished".format(num_finished, len(finished_list))
return not (False in finished_list)
Expand Down
10 changes: 9 additions & 1 deletion aiida/backends/sqlalchemy/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,19 @@
# For further information please visit http://www.aiida.net #
###########################################################################


# The next two serve as 'global' variables, set in the load_dbenv
# call. They are properly reset upon forking.
engine = None
scopedsessionclass = None

def get_scoped_session():
"""
Return a scoped session (according to SQLAlchemy docs,
this returns always the same object within a thread, and
a different object in a different thread.
Moreover, since we update the scopedsessionclass upon
forking, also forks have different session objects.
"""
if scopedsessionclass is None:
s = None
else:
Expand Down
16 changes: 5 additions & 11 deletions aiida/backends/sqlalchemy/globalsettings.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,9 @@

from aiida.backends.sqlalchemy.models.settings import DbSetting
from sqlalchemy.orm.exc import NoResultFound
from aiida.backends.sqlalchemy import get_scoped_session


def get_session():
"""
Return the global session for SQLA
"""
import aiida.backends.sqlalchemy
return aiida.backends.sqlalchemy.get_scoped_session()

def set_global_setting(key, value, description=None):
"""
Set a global setting in the DbSetting table (therefore, stored at the DB
Expand All @@ -39,7 +33,7 @@ def del_global_setting(key):
:raise KeyError: if the setting does not exist in the DB
"""
try:
setting = get_session().query(DbSetting).filter_by(key=key).one()
setting = get_scoped_session().query(DbSetting).filter_by(key=key).one()
setting.delete()
except NoResultFound:
raise KeyError("No global setting with key={}".format(key))
Expand All @@ -59,7 +53,7 @@ def get_global_setting(key):

try:
return get_value_of_sub_field(
key, lambda given_key: get_session().query(DbSetting).filter_by(
key, lambda given_key: get_scoped_session().query(DbSetting).filter_by(
key=given_key).one().getvalue())
except NoResultFound:
raise KeyError("No global setting with key={}".format(key))
Expand All @@ -78,7 +72,7 @@ def get_global_setting_description(key):
validate_key(key)

try:
return (get_session().query(DbSetting).filter_by(key=key).
return (get_scoped_session().query(DbSetting).filter_by(key=key).
one().get_description())
except NoResultFound:
raise KeyError("No global setting with key={}".format(key))
Expand All @@ -91,7 +85,7 @@ def table_check_test():
"""
from sqlalchemy.engine import reflection
from aiida.backends import sqlalchemy as sa
inspector = reflection.Inspector.from_engine(get_session().bind)
inspector = reflection.Inspector.from_engine(get_scoped_session().bind)
if 'db_dbsetting' not in inspector.get_table_names():
raise KeyError("No table found")

13 changes: 7 additions & 6 deletions aiida/backends/sqlalchemy/models/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,6 @@
from aiida.common.datastructures import calc_states


# Magic to make the most recent state given in DbCalcState an attribute
# of the DbNode (None if node is not a calculation)

class DbCalcState(Base):
__tablename__ = "db_dbcalcstate"

Expand All @@ -50,7 +47,6 @@ class DbCalcState(Base):
)
dbnode = relationship(
'DbNode', backref=backref('dbstates', passive_deletes=True),
#order_by='DbCalcState.time'
)

state = Column(ChoiceType((_, _) for _ in calc_states), index=True)
Expand Down Expand Up @@ -260,6 +256,9 @@ def __str__(self):

@hybrid_property
def state(self):
"""
Return the most recent state from DbCalcState
"""
if not self.id:
return None
all_states = DbCalcState.query.filter(DbCalcState.dbnode_id == self.id).all()
Expand All @@ -270,7 +269,10 @@ def state(self):

@state.expression
def state(cls):

"""
Return the expression to get the most recent state from DbCalcState,
to be used in queries
"""
subq = select(
[
DbCalcState.dbnode_id.label('dbnode_id'),
Expand All @@ -290,7 +292,6 @@ def state(cls):
label('laststate')



class DbLink(Base):
__tablename__ = "db_dblink"

Expand Down
4 changes: 2 additions & 2 deletions aiida/backends/sqlalchemy/querybuilder_sqla.py
Original file line number Diff line number Diff line change
Expand Up @@ -316,8 +316,8 @@ def get_filter_expr(

def _get_filter_expr_from_column(self, operator, value, column):

# Label is used because is what is returned for the
# 'state' column by the hybrid_column
# Label is used because it is what is returned for the
# 'state' column by the hybrid_column construct
if not isinstance(column, (Cast, InstrumentedAttribute, Label)):
raise TypeError(
'column ({}) {} is not a valid column'.format(
Expand Down
6 changes: 3 additions & 3 deletions aiida/backends/sqlalchemy/tests/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,16 @@ def test_deletion(self):

_ = JobCalculation(**calc_params).store()

#print "Node stored with pk:", _.dbnode.pk
session = aiida.backends.sqlalchemy.get_scoped_session()

# This should fail, because there is at least a calculation
# using this computer (the one created just above)
try:
aiida.backends.sqlalchemy.get_scoped_session().begin_nested()
session.begin_nested()
with self.assertRaises(InvalidOperation):
delete_computer(self.computer)
finally:
aiida.backends.sqlalchemy.get_scoped_session().rollback()
session.rollback()


class TestGroupsSqla(AiidaTestCase):
Expand Down
39 changes: 22 additions & 17 deletions aiida/backends/sqlalchemy/tests/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ def test_load_nodes(self):
from aiida.orm import load_node
from aiida.common.exceptions import NotExistent
import aiida.backends.sqlalchemy
from aiida.backends.sqlalchemy import get_scoped_session

a = Node()
a.store()
Expand All @@ -173,41 +174,43 @@ def test_load_nodes(self):
self.assertEquals(a.pk, load_node(node_id=a.uuid).pk)
self.assertEquals(a.pk, load_node(pk=a.pk).pk)
self.assertEquals(a.pk, load_node(uuid=a.uuid).pk)

session = get_scoped_session()

try:
aiida.backends.sqlalchemy.get_scoped_session().begin_nested()
session.begin_nested()
with self.assertRaises(ValueError):
load_node(node_id=a.pk, pk=a.pk)
finally:
aiida.backends.sqlalchemy.get_scoped_session().rollback()
session.rollback()

try:
aiida.backends.sqlalchemy.get_scoped_session().begin_nested()
session.begin_nested()
with self.assertRaises(ValueError):
load_node(pk=a.pk, uuid=a.uuid)
finally:
aiida.backends.sqlalchemy.get_scoped_session().rollback()
session.rollback()

try:
aiida.backends.sqlalchemy.get_scoped_session().begin_nested()
session.begin_nested()
with self.assertRaises(ValueError):
load_node(pk=a.uuid)
finally:
aiida.backends.sqlalchemy.get_scoped_session().rollback()
session.rollback()

try:
aiida.backends.sqlalchemy.get_scoped_session().begin_nested()
session.begin_nested()
with self.assertRaises(ValueError):
load_node(uuid=a.pk)
finally:
aiida.backends.sqlalchemy.get_scoped_session().rollback()
session.rollback()

try:
aiida.backends.sqlalchemy.get_scoped_session().begin_nested()
session.begin_nested()
with self.assertRaises(ValueError):
load_node()
finally:
aiida.backends.sqlalchemy.get_scoped_session().rollback()
session.rollback()

def test_multiple_node_creation(self):
"""
Expand All @@ -227,17 +230,19 @@ def test_multiple_node_creation(self):
node_uuid = get_new_uuid()
DbNode(user=user, uuid=node_uuid, type=None)

session = aiida.backends.sqlalchemy.get_scoped_session()

# Query the session before commit
res = aiida.backends.sqlalchemy.get_scoped_session().query(DbNode.uuid).filter(
res = session.query(DbNode.uuid).filter(
DbNode.uuid == node_uuid).all()
self.assertEqual(len(res), 0, "There should not be any nodes with this"
"UUID in the session/DB.")

# Commit the transaction
aiida.backends.sqlalchemy.get_scoped_session().commit()
session.commit()

# Check again that the node is not in the DB
res = aiida.backends.sqlalchemy.get_scoped_session().query(DbNode.uuid).filter(
res = session.query(DbNode.uuid).filter(
DbNode.uuid == node_uuid).all()
self.assertEqual(len(res), 0, "There should not be any nodes with this"
"UUID in the session/DB.")
Expand All @@ -247,20 +252,20 @@ def test_multiple_node_creation(self):
# Create a new node but now add it to the session
node_uuid = get_new_uuid()
node = DbNode(user=user, uuid=node_uuid, type=None)
aiida.backends.sqlalchemy.get_scoped_session().add(node)
session.add(node)

# Query the session before commit
res = aiida.backends.sqlalchemy.get_scoped_session().query(DbNode.uuid).filter(
res = session.query(DbNode.uuid).filter(
DbNode.uuid == node_uuid).all()
self.assertEqual(len(res), 1,
"There should be a node in the session/DB with the "
"UUID {}".format(node_uuid))

# Commit the transaction
aiida.backends.sqlalchemy.get_scoped_session().commit()
session.commit()

# Check again that the node is in the db
res = aiida.backends.sqlalchemy.get_scoped_session().query(DbNode.uuid).filter(
res = session.query(DbNode.uuid).filter(
DbNode.uuid == node_uuid).all()
self.assertEqual(len(res), 1,
"There should be a node in the session/DB with the "
Expand Down
23 changes: 15 additions & 8 deletions aiida/backends/sqlalchemy/tests/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,10 @@ def test_User_node_1(self):
self.assertIsNone(dbu1.id)
self.assertIsNone(dbn1.id)

session = aiida.backends.sqlalchemy.get_scoped_session()
# Add only the node and commit
aiida.backends.sqlalchemy.get_scoped_session().add(dbn1)
aiida.backends.sqlalchemy.get_scoped_session().commit()
session.add(dbn1)
session.commit()

# Check that a pk has been assigned, which means that things have
# been flushed into the database
Expand Down Expand Up @@ -162,13 +163,15 @@ def test_User_node_2(self):
self.assertIsNone(dbu1.id)
self.assertIsNone(dbn1.id)

session = aiida.backends.sqlalchemy.get_scoped_session()

# Catch all the SQLAlchemy warnings generated by the following code
with warnings.catch_warnings():
warnings.simplefilter("ignore", category=sa_exc.SAWarning)

# Add only the user and commit
aiida.backends.sqlalchemy.get_scoped_session().add(dbu1)
aiida.backends.sqlalchemy.get_scoped_session().commit()
session.add(dbu1)
session.commit()

# Check that a pk has been assigned (or not), which means that things
# have been flushed into the database
Expand Down Expand Up @@ -199,9 +202,11 @@ def test_User_node_3(self):
self.assertIsNone(dbn1.id)
self.assertIsNone(dbn2.id)

session = aiida.backends.sqlalchemy.get_scoped_session()

# Add only first node and commit
aiida.backends.sqlalchemy.get_scoped_session().add(dbn1)
aiida.backends.sqlalchemy.get_scoped_session().commit()
session.add(dbn1)
session.commit()

# Check for which object a pk has been assigned, which means that
# things have been at least flushed into the database
Expand Down Expand Up @@ -236,9 +241,11 @@ def test_User_node_4(self):
self.assertIsNone(dbu1.id)
self.assertIsNone(dbn1.id)

session = aiida.backends.sqlalchemy.get_scoped_session()

# Add only first node and commit
aiida.backends.sqlalchemy.get_scoped_session().add(dbn1)
aiida.backends.sqlalchemy.get_scoped_session().commit()
session.add(dbn1)
session.commit()

# Check for which object a pk has been assigned, which means that
# things have been at least flushed into the database
Expand Down
Loading

0 comments on commit 32cc238

Please sign in to comment.