From ac085f2fff59c40178daa4be3cf21c0002c39d77 Mon Sep 17 00:00:00 2001 From: Spyros Zoupanos Date: Mon, 13 Aug 2018 18:37:15 +0200 Subject: [PATCH] Addressing code review comments --- aiida/backends/tests/export_and_import.py | 5 -- aiida/cmdline/commands/exportfile.py | 28 ++++++----- aiida/cmdline/commands/importfile.py | 6 ++- aiida/orm/data/__init__.py | 2 - aiida/orm/importexport.py | 61 +++++++++++++---------- 5 files changed, 54 insertions(+), 48 deletions(-) diff --git a/aiida/backends/tests/export_and_import.py b/aiida/backends/tests/export_and_import.py index ac35998b19..0738000881 100644 --- a/aiida/backends/tests/export_and_import.py +++ b/aiida/backends/tests/export_and_import.py @@ -1808,11 +1808,6 @@ def test_links_for_workflows(self): o1.add_link_from(w1, 'output', link_type=LinkType.CREATE) o1.add_link_from(w1, 'return', link_type=LinkType.RETURN) - uuids_wanted = set(_.uuid for _ in (w1,o1,i1)) - # links_wanted = [l for l in self.get_all_node_links() if l[3] in - # ('createlink', 'inputlink', 'returnlink', 'calllink')] - # links_wanted = self.get_all_node_links() - links_wanted = [l for l in self.get_all_node_links() if l[3] in (LinkType.CREATE.value, LinkType.INPUT.value, diff --git a/aiida/cmdline/commands/exportfile.py b/aiida/cmdline/commands/exportfile.py index 6cf5868cf7..6331df86a8 100644 --- a/aiida/cmdline/commands/exportfile.py +++ b/aiida/cmdline/commands/exportfile.py @@ -39,20 +39,20 @@ def cli(self, *args): help='Export the given computers by pk') @click.option('-G', '--groups', multiple=True, type=int, help='Export the given groups by pk') -@click.option('-g', '--group_names', multiple=True, type=str, +@click.option('-g', '--group-names', multiple=True, type=str, help='Export the given groups by group name') -@click.option('-IF', '--input_forward', is_flag=True, default=False, - help='Follow forward INPUT links (recursively) when calculating the node ' - "set to export. By default this is switched off.") -@click.option('-CrR', '--create_reversed', is_flag=True, default=True, - help='Follow reverse CREATE links (recursively) when calculating the node ' - 'set to export. By default this is switched on.') -@click.option('-RR', '--return_reversed', is_flag=True, default=False, - help='Follow reverse RETURN links (recursively) when calculating the node ' - 'set to export. By default this is switched off.') -@click.option('-CaR', '--call_reversed', is_flag=True, default=False, - help='Follow reverse CALL links (recursively) when calculating the node ' - 'set to export. By default this is switched off.') +@click.option('-I', '--input-forward', is_flag=True, default=False, + show_default=True, help='Follow forward INPUT links (recursively) when ' + 'calculating the node set to export.') +@click.option('-C', '--create-reversed', is_flag=True, default=True, + show_default=True, help='Follow reverse CREATE links (recursively) when ' + 'calculating the node set to export.') +@click.option('-R', '--return-reversed', is_flag=True, default=False, + show_default=True, help='Follow reverse RETURN links (recursively) when ' + 'calculating the node set to export.') +@click.option('-X', '--call-reversed', is_flag=True, default=False, + show_default=True, help='Follow reverse CALL links (recursively) when ' + 'calculating the node set to export.') @click.option('-f', '--overwrite', is_flag=True, default=False, help='Overwrite the output file, if it exists') @click.option('-a', '--archive-format', @@ -66,6 +66,8 @@ def create(outfile, computers, groups, nodes, group_names, input_forward, """ import sys from aiida.backends.utils import load_dbenv, is_dbenv_loaded + # TODO: Replace with aiida.cmdline.utils.decorators.with_dbenv decocator + # TODO: when we merge to develop if not is_dbenv_loaded(): load_dbenv() from aiida.orm import Group, Node, Computer diff --git a/aiida/cmdline/commands/importfile.py b/aiida/cmdline/commands/importfile.py index 39bffce7df..759c0ea53b 100644 --- a/aiida/cmdline/commands/importfile.py +++ b/aiida/cmdline/commands/importfile.py @@ -31,6 +31,7 @@ def run(self, *args): from aiida.common.folders import SandboxFolder from aiida.orm.importexport import get_valid_import_links, import_data + from aiida.common.exceptions import ContentNotExistent parser = argparse.ArgumentParser( prog=self.get_full_command_name(), @@ -82,7 +83,10 @@ def run(self, *args): for filename in files: try: print "**** Importing file {}".format(filename) - import_data(filename) + try: + import_data(filename) + except ContentNotExistent as ce: + print ce except Exception: traceback.print_exc() diff --git a/aiida/orm/data/__init__.py b/aiida/orm/data/__init__.py index 0440b9fe42..86cbad1447 100644 --- a/aiida/orm/data/__init__.py +++ b/aiida/orm/data/__init__.py @@ -97,8 +97,6 @@ def add_link_from(self, src, label=None, link_type=LinkType.UNSPECIFIED): if link_type is LinkType.CREATE and \ len(self.get_inputs(link_type=LinkType.CREATE)) > 0: - for l in self.get_inputs(link_type=LinkType.CREATE): - print "=====> ", l, l.dbnode raise ValueError("At most one CREATE node can enter a data node") if not isinstance(src, Calculation): diff --git a/aiida/orm/importexport.py b/aiida/orm/importexport.py index d5dccdd2e2..b45c9a17da 100644 --- a/aiida/orm/importexport.py +++ b/aiida/orm/importexport.py @@ -387,9 +387,9 @@ def import_data_dj(in_path,ignore_unknown_nodes=False, "nor a zip file.") if not folder.get_content_list(): - print "The provided file/folder is empty. Exiting silently" - return - + from aiida.common.exceptions import ContentNotExistent + raise ContentNotExistent("The provided file/folder ({}) is empty" + .format(in_path)) try: with open(folder.get_abs_path('metadata.json')) as f: metadata = json.load(f) @@ -869,7 +869,6 @@ def import_data_sqla(in_path, ignore_unknown_nodes=False, silent=False): from aiida.orm import Node, Group from aiida.common.archive import extract_tree, extract_tar, extract_zip, extract_cif - from aiida.common.links import LinkType from aiida.common.folders import SandboxFolder, RepositoryFolder from aiida.common.utils import get_object_from_string from aiida.common.datastructures import calc_states @@ -910,6 +909,10 @@ def import_data_sqla(in_path, ignore_unknown_nodes=False, silent=False): "is neither a (possibly compressed) tar " "file, nor a zip file.") + if not folder.get_content_list(): + from aiida.common.exceptions import ContentNotExistent + raise ContentNotExistent("The provided file/folder ({}) is empty" + .format(in_path)) try: with open(folder.get_abs_path('metadata.json')) as f: metadata = json.load(f) @@ -1683,10 +1686,10 @@ def export_tree(what, folder,allowed_licenses=None, forbidden_licenses=None, silent=False, input_forward=False, create_reversed=True, return_reversed=False, call_reversed=False, **kwargs): """ - Export the DB entries passed in the 'what' list to a file tree. + Export the entries passed in the 'what' list to a file tree. :todo: limit the export to finished or failed calculations. - :param what: a list of Django database entries; they can belong to - different models. + :param what: a list of entity instances; they can belong to + different models/entities. :param folder: a :py:class:`Folder ` object :param input_forward: Follow forward INPUT links (recursively) when calculating the node set to export. @@ -1733,7 +1736,7 @@ def export_tree(what, folder,allowed_licenses=None, forbidden_licenses=None, given_node_entry_ids = set() given_group_entry_ids = set() - # I store a list of the actual dbnodes + # I keep the pks of the given entities for entry in what: if issubclass(entry.__class__, Group): given_group_entry_ids.add(entry.pk) @@ -1908,7 +1911,6 @@ def export_tree(what, folder,allowed_licenses=None, forbidden_licenses=None, else: curr_node_id = to_be_visited[Data].pop() # If it is already visited continue to the next node - # If it is already visited continue to the next node if curr_node_id in to_be_exported: continue # Otherwise say that it is a node to be exported @@ -2599,16 +2601,29 @@ def export_zip(what, outfile = 'testzip', overwrite = False, print "File written in {:10.3g} s.".format(time.time() - t) -def export(what, outfile='export_data.aiida.tar.gz', overwrite=False, silent=False, **kwargs): +def export(what, outfile='export_data.aiida.tar.gz', overwrite=False, + silent=False, **kwargs): """ - Export the DB entries passed in the 'what' list on a file. - + Export the entries passed in the 'what' list to a file tree. :todo: limit the export to finished or failed calculations. - - :param what: a list of Django database entries; they can belong to different - models. - :param also_parents: if True, also all the parents are stored (from the transitive closure) - :param also_calc_outputs: if True, any output of a calculation is also exported + :param what: a list of entity instances; they can belong to + different models/entities. + :param input_forward: Follow forward INPUT links (recursively) when + calculating the node set to export. + :param create_reversed: Follow reversed CREATE links (recursively) when + calculating the node set to export. + :param return_reversed: Follow reversed RETURN links (recursively) when + calculating the node set to export. + :param call_reversed: Follow reversed CALL links (recursively) when + calculating the node set to export. + :param allowed_licenses: a list or a function. If a list, then checks + whether all licenses of Data nodes are in the list. If a function, + then calls function for licenses of Data nodes expecting True if + license is allowed, False otherwise. + :param forbidden_licenses: a list or a function. If a list, then checks + whether all licenses of Data nodes are in the list. If a function, + then calls function for licenses of Data nodes expecting True if + license is allowed, False otherwise. :param outfile: the filename of the file on which to export :param overwrite: if True, overwrite the output file without asking. if False, raise an IOError in this case. @@ -2635,25 +2650,17 @@ def export(what, outfile='export_data.aiida.tar.gz', overwrite=False, silent=Fal if not silent: print "COMPRESSING..." - # PAX_FORMAT: virtually no limitations, better support for unicode - # characters - # dereference=True: at the moment, we should not have any symlink or - # hardlink in the AiiDA repository; therefore, do not store symlinks - # or hardlinks, but store the actual destinations. - # This also simplifies the checks on import. t3 = time.time() with tarfile.open(outfile, "w:gz", format=tarfile.PAX_FORMAT, dereference=True) as tar: tar.add(folder.abspath, arcname="") - - # import shutil - # shutil.make_archive(outfile, 'zip', folder.abspath)#, base_dir='aiida') t4 = time.time() if not silent: filecr_time = t2-t1 filecomp_time = t4-t3 - print "Exported in {:6.2g}s, compressed in {:6.2g}s, total: {:6.2g}s.".format(filecr_time, filecomp_time, filecr_time + filecomp_time) + print("Exported in {:6.2g}s, compressed in {:6.2g}s, total: {:6.2g}s." + .format(filecr_time, filecomp_time, filecr_time + filecomp_time)) if not silent: print "DONE."