From 7a5b3afe4caf7060e4944506443901b37dcdd8ec Mon Sep 17 00:00:00 2001 From: Spyros Zoupanos Date: Wed, 4 Oct 2017 14:22:15 +0200 Subject: [PATCH 1/5] Reproducing the problem --- aiida/backends/tests/export_and_import.py | 82 +++++++++++++++++++++++ aiida/orm/importexport.py | 2 +- 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/aiida/backends/tests/export_and_import.py b/aiida/backends/tests/export_and_import.py index 1269cc7fba..c81b1d08cc 100644 --- a/aiida/backends/tests/export_and_import.py +++ b/aiida/backends/tests/export_and_import.py @@ -1166,3 +1166,85 @@ def test_different_computer_same_name_import(self): shutil.rmtree(export_file_tmp_folder, ignore_errors=True) shutil.rmtree(unpack_tmp_folder, ignore_errors=True) + + def test_correct_import_of_computer_json_params(self): + """ + This test checks that if there is a name collision, the imported + computers are renamed accordingly. + """ + import os + import shutil + import tempfile + + from aiida.orm.importexport import export + from aiida.orm.querybuilder import QueryBuilder + from aiida.orm.computer import Computer + from aiida.orm.calculation.job import JobCalculation + from aiida.orm.importexport import COMP_DUPL_SUFFIX + + # Creating a folder for the import/export files + export_file_tmp_folder = tempfile.mkdtemp() + unpack_tmp_folder = tempfile.mkdtemp() + + try: + # Set the computer name + comp1_name = "localhost_1" + self.computer.set_name(comp1_name) + self.computer.set_transport_params("{}") + # self.computer._set_metadata("{}") + + # Store a calculation + calc1_label = "calc1" + calc1 = JobCalculation() + calc1.set_computer(self.computer) + calc1.set_resources({"num_machines": 1, + "num_mpiprocs_per_machine": 1}) + calc1.label = calc1_label + calc1.store() + calc1._set_state(u'RETRIEVING') + + qb = QueryBuilder() + qb.append(JobCalculation, project=['label'], tag='jcalc') + qb.append(Computer, project=['name', 'transport_params', '_metadata'], + computer_of='jcalc') + res = qb.all() + + print "==================================================" + print res + print "==================================================" + + # Export the first job calculation + filename1 = os.path.join(export_file_tmp_folder, "export1.tar.gz") + export([calc1.dbnode], outfile=filename1, silent=True) + + # Reset the database + self.clean_db() + + # Import all the calculation + import_data(filename1, silent=True) + + # Retrieve the calculation-computer pairs + qb = QueryBuilder() + qb.append(JobCalculation, project=['label'], tag='jcalc') + qb.append(Computer, project=['name', 'transport_params', '_metadata'], + computer_of='jcalc') + # self.assertEqual(qb.count(), 3, "Three combinations expected.") + res = qb.all() + + print "==================================================" + print res + print "==================================================" + + # self.assertIn([calc1_label, comp1_name], res, + # "Calc-Computer combination not found.") + # self.assertIn([calc2_label, + # comp1_name + COMP_DUPL_SUFFIX.format(0)], res, + # "Calc-Computer combination not found.") + # self.assertIn([calc3_label, + # comp1_name + COMP_DUPL_SUFFIX.format(1)], res, + # "Calc-Computer combination not found.") + finally: + # Deleting the created temporary folders + shutil.rmtree(export_file_tmp_folder, ignore_errors=True) + shutil.rmtree(unpack_tmp_folder, ignore_errors=True) + diff --git a/aiida/orm/importexport.py b/aiida/orm/importexport.py index ed81272f82..efcbfb5699 100644 --- a/aiida/orm/importexport.py +++ b/aiida/orm/importexport.py @@ -2353,7 +2353,7 @@ def export_tree(what, folder, also_parents = True, also_calc_outputs=True, forbidden_licenses=forbidden_licenses, silent=silent) elif BACKEND == BACKEND_DJANGO: - export_tree_dj(what, folder, also_parents = also_parents, + export_tree_sqla(what, folder, also_parents = also_parents, also_calc_outputs=also_calc_outputs, allowed_licenses=allowed_licenses, forbidden_licenses=forbidden_licenses, From e7efa445c750343c4646046f6e69209b1c4fb88c Mon Sep 17 00:00:00 2001 From: Spyros Zoupanos Date: Wed, 4 Oct 2017 15:31:16 +0200 Subject: [PATCH 2/5] Adding test that demonstrates the problem --- aiida/backends/tests/query.py | 76 +++++++++++++++++------------------ 1 file changed, 36 insertions(+), 40 deletions(-) diff --git a/aiida/backends/tests/query.py b/aiida/backends/tests/query.py index d4b2efc5df..e0aa2b3f2b 100644 --- a/aiida/backends/tests/query.py +++ b/aiida/backends/tests/query.py @@ -13,8 +13,6 @@ import aiida.backends.settings as settings - - class TestQueryBuilder(AiidaTestCase): def test_classification(self): @@ -70,7 +68,6 @@ def test_classification(self): self.assertEqual(clstype, 'user') self.assertEqual(query_type_string, None) - for cls, clstype, query_type_string in ( qb._get_ormclass(Computer, None), qb._get_ormclass(None, 'computer'), @@ -79,7 +76,6 @@ def test_classification(self): self.assertEqual(clstype, 'computer') self.assertEqual(query_type_string, None) - for cls, clstype, query_type_string in ( qb._get_ormclass(Data, None), qb._get_ormclass(None, 'data.Data.'), @@ -87,7 +83,6 @@ def test_classification(self): self.assertEqual(clstype, Data._plugin_type_string) self.assertEqual(query_type_string, Data._query_type_string) - def test_simple_query_1(self): """ Testing a simple query @@ -162,7 +157,6 @@ def test_simple_query_1(self): qb6.append(Data, tag='node2') self.assertEqual(qb6.count(), 0) - def test_simple_query_2(self): from aiida.orm.querybuilder import QueryBuilder from aiida.orm import Node @@ -187,13 +181,10 @@ def test_simple_query_2(self): for n in (n0, n1, n2): n.store() - - qb1 = QueryBuilder() qb1.append(Node, filters={'label': 'hello'}) self.assertEqual(len(list(qb1.all())), 1) - qh = { 'path': [ { @@ -222,18 +213,13 @@ def test_simple_query_2(self): qb2 = QueryBuilder(**qh) - resdict = qb2.dict() self.assertEqual(len(resdict), 1) self.assertTrue(isinstance(resdict[0]['n1']['ctime'], datetime)) - res_one = qb2.one() self.assertTrue('bar' in res_one) - - - qh = { 'path': [ { @@ -253,7 +239,6 @@ def test_simple_query_2(self): qb = QueryBuilder(**qh) self.assertEqual(qb.count(), 1) - # Test the hashing: query1 = qb.get_query() qb.add_filter('n2', {'label': 'nonexistentlabel'}) @@ -270,7 +255,6 @@ def test_simple_query_2(self): self.assertTrue(id(query1) != id(query2)) self.assertTrue(id(query2) == id(query3)) - def test_operators_eq_lt_gt(self): from aiida.orm.querybuilder import QueryBuilder from aiida.orm import Node @@ -297,9 +281,6 @@ def test_operators_eq_lt_gt(self): self.assertEqual(QueryBuilder().append(Node, filters={'attributes.fa':{'>':1.02}}).count(), 4) self.assertEqual(QueryBuilder().append(Node, filters={'attributes.fa':{'>=':1.02}}).count(), 5) - - - def test_subclassing(self): from aiida.orm.data.structure import StructureData from aiida.orm.data.parameter import ParameterData @@ -337,7 +318,6 @@ def test_subclassing(self): qb = QueryBuilder().append(cls, filters={'attributes.cat':'miau'}, subclassing=False) self.assertEqual(qb.count(), 1) - def test_list_behavior(self): from aiida.orm import Node from aiida.orm.querybuilder import QueryBuilder @@ -388,8 +368,6 @@ def test_append_validation(self): with self.assertRaises(InputValidationError): QueryBuilder(offset=2.3) - - # So, I mess up one append, I want the QueryBuilder to clean it! with self.assertRaises(InputValidationError): qb = QueryBuilder() @@ -441,6 +419,42 @@ def test_tags(self): ]) +class TestQueryBuilderCornerCases(AiidaTestCase): + + def test_computer_json(self): + """ + Testing a simple query + """ + from aiida.orm.querybuilder import QueryBuilder + from aiida.orm import Node, Data, Calculation + from aiida.orm import Computer + + n1 = Calculation() + n1.label = 'node2' + n1._set_attr('foo', 1) + n1.store() + + # Checking the correct retrieval of transport_params which is + # a JSON field (in both backends). + qb = QueryBuilder() + qb.append(Calculation, project=['id'], tag='calc') + # qb.append(Computer, project=['id', '_metadata', 'transport_params'], outerjoin=True, computer_of='calc') + qb.append(Computer, project=['id', 'transport_params'], + outerjoin=True, computer_of='calc') + print "=======>", qb.all() + + # Checking the correct retrieval of _metadata which is + # a JSON field (in both backends). + qb = QueryBuilder() + qb.append(Calculation, project=['id'], tag='calc') + # qb.append(Computer, project=['id', '_metadata', 'transport_params'], outerjoin=True, computer_of='calc') + qb.append(Computer, project=['id', '_metadata'], + outerjoin=True, computer_of='calc') + print "=======>", qb.all() + + + + class TestAttributes(AiidaTestCase): def test_attribute_existence(self): # I'm storing a value under key whatever: @@ -453,21 +467,6 @@ def test_attribute_existence(self): n1._set_attr("test_case", "test_attribute_existence") n1.store() - n1 = Node() - n1._set_attr("whatever", 0.) - n1._set_attr("test_case", "test_attribute_existence") - n1.store() - res_uuids.add(n1.uuid) - - n1 = Node() - n1._set_attr("whatever", None) - n1._set_attr("test_case", "test_attribute_existence") - n1.store() - - n1 = Node() - n1._set_attr("test_case", "test_attribute_existence") - n1.store() - res_uuids.add(n1.uuid) # I want all the nodes where whatever is smaller than 1. or there is no such value: @@ -482,7 +481,6 @@ def test_attribute_existence(self): self.assertEqual(res_query, res_uuids) - class QueryBuilderDateTimeAttribute(AiidaTestCase): @unittest.skipIf(settings.BACKEND == u'sqlalchemy', "SQLA doesn't have full datetime support in attributes") @@ -504,8 +502,6 @@ def test_date(self): self.assertEqual(qb.count(), 1) - - class QueryBuilderLimitOffsetsTest(AiidaTestCase): def test_ordering_limits_offsets_of_results_general(self): From 835ea5e16abe62eff2ffc9603cc7b73966681c22 Mon Sep 17 00:00:00 2001 From: Spyros Zoupanos Date: Wed, 4 Oct 2017 15:46:34 +0200 Subject: [PATCH 3/5] Doing the fix and cleaning up. --- .../querybuilder_django/querybuilder_django.py | 2 +- aiida/backends/tests/query.py | 17 ++++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/aiida/backends/djsite/querybuilder_django/querybuilder_django.py b/aiida/backends/djsite/querybuilder_django/querybuilder_django.py index 0a1fbf0aff..4694eda1b5 100644 --- a/aiida/backends/djsite/querybuilder_django/querybuilder_django.py +++ b/aiida/backends/djsite/querybuilder_django/querybuilder_django.py @@ -512,7 +512,7 @@ def get_aiida_res(self, key, res): elif key == 'extras': # same as attributes return DbExtra.get_all_values_for_nodepk(res) - elif key in ('_metadata', 'transport_params'): + elif key in ('_metadata', 'transport_params') and res is not None: # Metadata and transport_params are stored as json strings in the DB: return json_loads(res) elif isinstance(res, (self.Group, self.Node, self.Computer, self.User)): diff --git a/aiida/backends/tests/query.py b/aiida/backends/tests/query.py index e0aa2b3f2b..f5f9440fc3 100644 --- a/aiida/backends/tests/query.py +++ b/aiida/backends/tests/query.py @@ -420,10 +420,17 @@ def test_tags(self): class TestQueryBuilderCornerCases(AiidaTestCase): + """ + In this class corner cases of QueryBuilder are added. + """ def test_computer_json(self): """ - Testing a simple query + In this test we check the correct behavior of QueryBuilder when + retrieving the _metadata and the transport_params with no content. + Note that they are in JSON format in both backends. Forcing the + decoding of a None value leads to an exception (this was the case + under Django). """ from aiida.orm.querybuilder import QueryBuilder from aiida.orm import Node, Data, Calculation @@ -438,21 +445,17 @@ def test_computer_json(self): # a JSON field (in both backends). qb = QueryBuilder() qb.append(Calculation, project=['id'], tag='calc') - # qb.append(Computer, project=['id', '_metadata', 'transport_params'], outerjoin=True, computer_of='calc') qb.append(Computer, project=['id', 'transport_params'], outerjoin=True, computer_of='calc') - print "=======>", qb.all() + qb.all() # Checking the correct retrieval of _metadata which is # a JSON field (in both backends). qb = QueryBuilder() qb.append(Calculation, project=['id'], tag='calc') - # qb.append(Computer, project=['id', '_metadata', 'transport_params'], outerjoin=True, computer_of='calc') qb.append(Computer, project=['id', '_metadata'], outerjoin=True, computer_of='calc') - print "=======>", qb.all() - - + qb.all() class TestAttributes(AiidaTestCase): From 1140fbe4246101d6b4499a9e5ad4c592cfb77c80 Mon Sep 17 00:00:00 2001 From: Spyros Zoupanos Date: Wed, 4 Oct 2017 15:51:58 +0200 Subject: [PATCH 4/5] Removing not needed test. --- aiida/backends/tests/export_and_import.py | 83 ----------------------- 1 file changed, 83 deletions(-) diff --git a/aiida/backends/tests/export_and_import.py b/aiida/backends/tests/export_and_import.py index c81b1d08cc..1bcb8f0030 100644 --- a/aiida/backends/tests/export_and_import.py +++ b/aiida/backends/tests/export_and_import.py @@ -1165,86 +1165,3 @@ def test_different_computer_same_name_import(self): # Deleting the created temporary folders shutil.rmtree(export_file_tmp_folder, ignore_errors=True) shutil.rmtree(unpack_tmp_folder, ignore_errors=True) - - - def test_correct_import_of_computer_json_params(self): - """ - This test checks that if there is a name collision, the imported - computers are renamed accordingly. - """ - import os - import shutil - import tempfile - - from aiida.orm.importexport import export - from aiida.orm.querybuilder import QueryBuilder - from aiida.orm.computer import Computer - from aiida.orm.calculation.job import JobCalculation - from aiida.orm.importexport import COMP_DUPL_SUFFIX - - # Creating a folder for the import/export files - export_file_tmp_folder = tempfile.mkdtemp() - unpack_tmp_folder = tempfile.mkdtemp() - - try: - # Set the computer name - comp1_name = "localhost_1" - self.computer.set_name(comp1_name) - self.computer.set_transport_params("{}") - # self.computer._set_metadata("{}") - - # Store a calculation - calc1_label = "calc1" - calc1 = JobCalculation() - calc1.set_computer(self.computer) - calc1.set_resources({"num_machines": 1, - "num_mpiprocs_per_machine": 1}) - calc1.label = calc1_label - calc1.store() - calc1._set_state(u'RETRIEVING') - - qb = QueryBuilder() - qb.append(JobCalculation, project=['label'], tag='jcalc') - qb.append(Computer, project=['name', 'transport_params', '_metadata'], - computer_of='jcalc') - res = qb.all() - - print "==================================================" - print res - print "==================================================" - - # Export the first job calculation - filename1 = os.path.join(export_file_tmp_folder, "export1.tar.gz") - export([calc1.dbnode], outfile=filename1, silent=True) - - # Reset the database - self.clean_db() - - # Import all the calculation - import_data(filename1, silent=True) - - # Retrieve the calculation-computer pairs - qb = QueryBuilder() - qb.append(JobCalculation, project=['label'], tag='jcalc') - qb.append(Computer, project=['name', 'transport_params', '_metadata'], - computer_of='jcalc') - # self.assertEqual(qb.count(), 3, "Three combinations expected.") - res = qb.all() - - print "==================================================" - print res - print "==================================================" - - # self.assertIn([calc1_label, comp1_name], res, - # "Calc-Computer combination not found.") - # self.assertIn([calc2_label, - # comp1_name + COMP_DUPL_SUFFIX.format(0)], res, - # "Calc-Computer combination not found.") - # self.assertIn([calc3_label, - # comp1_name + COMP_DUPL_SUFFIX.format(1)], res, - # "Calc-Computer combination not found.") - finally: - # Deleting the created temporary folders - shutil.rmtree(export_file_tmp_folder, ignore_errors=True) - shutil.rmtree(unpack_tmp_folder, ignore_errors=True) - From fefdf5ceee53737dde1891b5c02ddd1f4c14ac31 Mon Sep 17 00:00:00 2001 From: Spyros Zoupanos Date: Wed, 4 Oct 2017 15:53:35 +0200 Subject: [PATCH 5/5] Removing not needed changes at importexport.py --- aiida/orm/importexport.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiida/orm/importexport.py b/aiida/orm/importexport.py index efcbfb5699..ed81272f82 100644 --- a/aiida/orm/importexport.py +++ b/aiida/orm/importexport.py @@ -2353,7 +2353,7 @@ def export_tree(what, folder, also_parents = True, also_calc_outputs=True, forbidden_licenses=forbidden_licenses, silent=silent) elif BACKEND == BACKEND_DJANGO: - export_tree_sqla(what, folder, also_parents = also_parents, + export_tree_dj(what, folder, also_parents = also_parents, also_calc_outputs=also_calc_outputs, allowed_licenses=allowed_licenses, forbidden_licenses=forbidden_licenses,