Skip to content

Commit

Permalink
Fix bug in SqlAlchemy migration `7ca08c391c49_calc_job_option_attribu…
Browse files Browse the repository at this point in the history
…te_keys`

The migration incorrectly converted the dictionary attributes with the
new keys `resources` and `environment_variables` to text, because the
wrong operator `->>` was used. This operator gets the JSONB value as
text, whereas `->` keeps the type, which is the one that should have
been used. This was missed by the tests, because the tests undid the
type conversion using `ast`. Collossal fail...
  • Loading branch information
sphuber committed May 3, 2019
1 parent 9b4c54c commit f079389
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def upgrade():

statement = text("""
UPDATE db_dbnode
SET attributes = jsonb_set(attributes, '{environment_variables}', to_jsonb(attributes->>'custom_environment_variables'))
SET attributes = jsonb_set(attributes, '{environment_variables}', attributes->'custom_environment_variables')
WHERE
attributes ? 'custom_environment_variables' AND
type = 'node.process.calculation.calcjob.CalcJobNode.';
Expand All @@ -60,7 +60,7 @@ def upgrade():
-- custom_environment_variables -> environment_variables
UPDATE db_dbnode
SET attributes = jsonb_set(attributes, '{resources}', to_jsonb(attributes->>'jobresource_params'))
SET attributes = jsonb_set(attributes, '{resources}', attributes->'jobresource_params')
WHERE
attributes ? 'jobresource_params' AND
type = 'node.process.calculation.calcjob.CalcJobNode.';
Expand All @@ -71,7 +71,7 @@ def upgrade():
-- jobresource_params -> resources
UPDATE db_dbnode
SET attributes = jsonb_set(attributes, '{process_label}', to_jsonb(attributes->>'_process_label'))
SET attributes = jsonb_set(attributes, '{process_label}', attributes->'_process_label')
WHERE
attributes ? '_process_label' AND
type like 'node.process.%';
Expand All @@ -82,7 +82,7 @@ def upgrade():
-- _process_label -> process_label
UPDATE db_dbnode
SET attributes = jsonb_set(attributes, '{parser_name}', to_jsonb(attributes->>'parser'))
SET attributes = jsonb_set(attributes, '{parser_name}', attributes->'parser')
WHERE
attributes ? 'parser' AND
type = 'node.process.calculation.calcjob.CalcJobNode.';
Expand Down
8 changes: 2 additions & 6 deletions aiida/backends/sqlalchemy/tests/test_migrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,6 @@ def setUpBeforeMigration(self):

def test_attribute_key_changes(self):
"""Verify that the keys are successfully changed of the affected attributes."""
import ast
from sqlalchemy.orm import Session # pylint: disable=import-error,no-name-in-module

DbNode = self.get_auto_base().classes.db_dbnode # pylint: disable=invalid-name
Expand All @@ -590,15 +589,12 @@ def test_attribute_key_changes(self):
self.assertEqual(node_work.attributes.get(self.KEY_PROCESS_LABEL_NEW), self.process_label)
self.assertEqual(node_work.attributes.get(self.KEY_PROCESS_LABEL_OLD, not_found), not_found)

# The dictionaries need to be cast with ast.literal_eval, because the `get` will return a string
# representation of the dictionary that the attribute contains
node_calc = session.query(DbNode).filter(DbNode.id == self.node_calc_id).one()
self.assertEqual(node_calc.attributes.get(self.KEY_PROCESS_LABEL_NEW), self.process_label)
self.assertEqual(node_calc.attributes.get(self.KEY_PARSER_NAME_NEW), self.parser_name)
self.assertEqual(ast.literal_eval(node_calc.attributes.get(self.KEY_RESOURCES_NEW)), self.resources)
self.assertEqual(node_calc.attributes.get(self.KEY_RESOURCES_NEW), self.resources)
self.assertEqual(
ast.literal_eval(node_calc.attributes.get(self.KEY_ENVIRONMENT_VARIABLES_NEW)),
self.environment_variables)
node_calc.attributes.get(self.KEY_ENVIRONMENT_VARIABLES_NEW), self.environment_variables)
self.assertEqual(node_calc.attributes.get(self.KEY_PROCESS_LABEL_OLD, not_found), not_found)
self.assertEqual(node_calc.attributes.get(self.KEY_PARSER_NAME_OLD, not_found), not_found)
self.assertEqual(node_calc.attributes.get(self.KEY_RESOURCES_OLD, not_found), not_found)
Expand Down

0 comments on commit f079389

Please sign in to comment.