Merge pull request #51 from datmo/issue-17

fix for issue #17

datmo/cli/command/snapshot.py

@@ -3,6 +3,7 @@
 import prettytable
 
 from datmo.core.util.i18n import get as __
+from datmo.core.util.misc_functions import mutually_exclusive
 from datmo.cli.command.project import ProjectCommand
 from datmo.core.controller.snapshot import SnapshotController
 
@@ -15,36 +16,39 @@ def __init__(self, home, cli_helper):
         snapshot_parser = self.subparsers.add_parser("snapshot", help="Snapshot module")
         subcommand_parsers = snapshot_parser.add_subparsers(title="subcommands", dest="subcommand")
 
-        create = subcommand_parsers.add_parser("create", help="Create snapshot")
-        create.add_argument("--message", "-m", dest="message", default="", help="Message to describe snapshot")
-        create.add_argument("--label", "-l", dest="label", default="",
+        create = subcommand_parsers.add_parser("create", help="create snapshot")
+        create.add_argument("--message", "-m", dest="message", default=None, help="message to describe snapshot")
+        create.add_argument("--label", "-l", dest="label", default=None,
                             help="Label snapshots with a category (e.g. best)")
-        create.add_argument("--session-id", dest="session_id", default="", help="User given session id")
+        create.add_argument("--session-id", dest="session_id", default=None, help="user given session id")
 
-        create.add_argument("--task-id", dest="task_id", default="",
+        create.add_argument("--task-id", dest="task_id", default=None,
                             help="Specify task id to pull information from")
 
-        create.add_argument("--code-id", dest="code_id", default="",
-                            help="User provided code id (e.g. git revision for git)")
+        create.add_argument("--code-id", dest="code_id", default=None,
+                            help="code id from code object")
+        create.add_argument("--commit-id", dest="commit_id", default=None,
+                            help="commit id from source control")
 
-        create.add_argument("--environment-def-path", dest="environment_def_path", default="",
-                            help="Absolute filepath to environment definition file (e.g. /path/to/Dockerfile)")
+        create.add_argument("--environment-id", dest="environment_id", default=None,
+                            help="environment id from environment object")
+        create.add_argument("--environment-def-path", dest="environment_def_path", default=None,
+                            help="absolute filepath to environment definition file (e.g. /path/to/Dockerfile)")
 
-        create.add_argument("--config-filename", dest="config_filename", default="",
-                            help="Filename to use to search for configuration JSON")
-        create.add_argument("--config-filepath", dest="config_filepath", default="",
-                            help="Absolute filepath to use to search for configuration JSON")
+        create.add_argument("--file-collection-id", dest="file_collection_id", default=None,
+                            help="file collection id for file collection object")
+        create.add_argument("--filepaths", dest="filepaths", default=None, nargs="*",
+                            help="absolute paths to files or folders to include within the files of the snapshot")
 
-        create.add_argument("--stats-filename", dest="stats_filename", default="",
-                            help="Filename to use to search for metrics JSON")
-        create.add_argument("--stats-filepath", dest="stats_filepath", default="",
-                            help="Absolute filepath to use to search for metrics JSON")
+        create.add_argument("--config-filename", dest="config_filename", default=None,
+                            help="filename to use to search for configuration JSON")
+        create.add_argument("--config-filepath", dest="config_filepath", default=None,
+                            help="absolute filepath to use to search for configuration JSON")
 
-        create.add_argument("--filepaths", dest="filepaths", default=[], nargs="*",
-                            help="Absolute paths to files or folders to include within the files of the snapshot")
-
-        create.add_argument("--not-visible", dest="visible", action="store_false",
-                         help="Boolean if you want snapshot to not be visible")
+        create.add_argument("--stats-filename", dest="stats_filename", default=None,
+                            help="filename to use to search for metrics JSON")
+        create.add_argument("--stats-filepath", dest="stats_filepath", default=None,
+                            help="absolute filepath to use to search for metrics JSON")
 
         delete = subcommand_parsers.add_parser("delete", help="Delete a snapshot by id")
         delete.add_argument("--id", dest="id", help="snapshot id to delete")
@@ -62,39 +66,32 @@ def __init__(self, home, cli_helper):
     def create(self, **kwargs):
         self.cli_helper.echo(__("info", "cli.snapshot.create"))
 
-        def mutually_exclusive(dictionary, mutually_exclusive_args):
-            for arg in mutually_exclusive_args:
-                if arg in kwargs and kwargs[arg]:
-                    snapshot_dict[arg] = kwargs[arg]
-                    break
-            return dictionary
-
         snapshot_dict = {}
 
         # Code
         mutually_exclusive_args = ["code_id", "commit_id"]
-        snapshot_dict = mutually_exclusive(snapshot_dict, mutually_exclusive_args)
+        mutually_exclusive(mutually_exclusive_args, kwargs, snapshot_dict)
 
         # Environment
-        mutually_exclusive_args = ["environment_id", "environment_definition_filepath"]
-        snapshot_dict = mutually_exclusive(snapshot_dict, mutually_exclusive_args)
+        mutually_exclusive_args = ["environment_id", "environment_def_path"]
+        mutually_exclusive(mutually_exclusive_args, kwargs, snapshot_dict)
 
         # File
-        mutually_exclusive_args = ["file_collection_id", "file_collection"]
-        snapshot_dict = mutually_exclusive(snapshot_dict, mutually_exclusive_args)
+        mutually_exclusive_args = ["file_collection_id", "filepaths"]
+        mutually_exclusive(mutually_exclusive_args, kwargs, snapshot_dict)
 
         # Config
         mutually_exclusive_args = ["config_filepath", "config_filename"]
-        snapshot_dict = mutually_exclusive(snapshot_dict, mutually_exclusive_args)
+        mutually_exclusive(mutually_exclusive_args, kwargs, snapshot_dict)
 
         # Stats
         mutually_exclusive_args = ["stats_filepath", "stats_filename"]
-        snapshot_dict = mutually_exclusive(snapshot_dict, mutually_exclusive_args)
+        mutually_exclusive(mutually_exclusive_args, kwargs, snapshot_dict)
 
-        optional_args = ["session_id", "task_id", "message", "label", "visible"]
+        optional_args = ["session_id", "task_id", "message", "label"]
 
         for arg in optional_args:
-            if arg in kwargs and kwargs[arg]:
+            if arg in kwargs and kwargs[arg] is not None:
                 snapshot_dict[arg] = kwargs[arg]
 
         snapshot_obj = self.snapshot_controller.create(snapshot_dict)

datmo/cli/command/test/test_snapshot.py

@@ -12,7 +12,6 @@
 #     # Python 3
 #     import builtins as __builtin__
 import os
-import shutil
 import tempfile
 import platform
 from io import open
@@ -24,7 +23,8 @@
 from datmo.cli.driver.helper import Helper
 from datmo.cli.command.project import ProjectCommand
 from datmo.cli.command.snapshot import SnapshotCommand
-from datmo.core.util.exceptions import ProjectNotInitializedException
+from datmo.core.util.exceptions import ProjectNotInitializedException, \
+    MutuallyExclusiveArguments
 
 
 class TestSnapshot():
@@ -88,12 +88,9 @@ def test_datmo_snapshot_create(self):
         test_task_id = "test_task_id"
         test_code_id = "test_code_id"
         test_environment_def_path = self.env_def_path
-        test_config_filename = "config.json"
         test_config_filepath = self.config_filepath
-        test_stats_filename = "stats.json"
         test_stats_filepath = self.config_filepath
         test_filepaths = [self.filepath]
-        test_visible = False
 
         self.snapshot.parse([
             "snapshot",
@@ -104,12 +101,9 @@ def test_datmo_snapshot_create(self):
             "--task-id", test_task_id,
             "--code-id", test_code_id,
             "--environment-def-path", test_environment_def_path,
-            "--config-filename", test_config_filename,
             "--config-filepath", test_config_filepath,
-            "--stats-filename", test_stats_filename,
             "--stats-filepath", test_stats_filepath,
             "--filepaths", test_filepaths[0],
-            "--not-visible"
         ])
 
         # test for desired side effects
@@ -119,17 +113,123 @@ def test_datmo_snapshot_create(self):
         assert self.snapshot.args.task_id == test_task_id
         assert self.snapshot.args.code_id == test_code_id
         assert self.snapshot.args.environment_def_path == test_environment_def_path
-        assert self.snapshot.args.config_filename == test_config_filename
         assert self.snapshot.args.config_filepath == test_config_filepath
-        assert self.snapshot.args.stats_filename == test_stats_filename
         assert self.snapshot.args.stats_filepath == test_stats_filepath
         assert self.snapshot.args.filepaths == test_filepaths
-        assert self.snapshot.args.visible == test_visible
 
         snapshot_id_1 = self.snapshot.execute()
         assert snapshot_id_1
 
-        # Test when optional parameters are not given
+    def test_datmo_snapshot_create_fail_mutually_exclusive_args(self):
+        self.__set_variables()
+        test_message = "this is a test message"
+        test_label = "test label"
+        test_session_id = "test_session_id"
+        test_task_id = "test_task_id"
+        test_code_id = "test_code_id"
+        test_commit_id = "test_commit_id"
+        test_environment_id = "test_environment_id"
+        test_environment_def_path = self.env_def_path
+        test_file_collection_id = "test_file_collection_id"
+        test_filepaths = [self.filepath]
+        test_config_filename = "config.json"
+        test_config_filepath = self.config_filepath
+        test_stats_filename = "stats.json"
+        test_stats_filepath = self.config_filepath
+
+        # Code exception
+        exception_thrown = False
+        try:
+            self.snapshot.parse([
+                "snapshot",
+                "create",
+                "--message", test_message,
+                "--label", test_label,
+                "--session-id", test_session_id,
+                "--task-id", test_task_id,
+                "--code-id", test_code_id,
+                "--commit-id", test_commit_id
+            ])
+            _ = self.snapshot.execute()
+        except MutuallyExclusiveArguments:
+            exception_thrown = True
+        assert exception_thrown
+
+        # Environment exception
+        exception_thrown = False
+        try:
+            self.snapshot.parse([
+                "snapshot",
+                "create",
+                "--message", test_message,
+                "--label", test_label,
+                "--session-id", test_session_id,
+                "--task-id", test_task_id,
+                "--environment-id", test_environment_id,
+                "--environment-def-path", test_environment_def_path,
+            ])
+            _ = self.snapshot.execute()
+        except MutuallyExclusiveArguments:
+            exception_thrown = True
+        assert exception_thrown
+
+        # File exception
+        exception_thrown = False
+        try:
+            self.snapshot.parse([
+                "snapshot",
+                "create",
+                "--message", test_message,
+                "--label", test_label,
+                "--session-id", test_session_id,
+                "--task-id", test_task_id,
+                "--file-collection-id", test_file_collection_id,
+                "--filepaths", test_filepaths[0],
+            ])
+            _ = self.snapshot.execute()
+        except MutuallyExclusiveArguments:
+            exception_thrown = True
+        assert exception_thrown
+
+        # Config exception
+        exception_thrown = False
+        try:
+            self.snapshot.parse([
+                "snapshot",
+                "create",
+                "--message", test_message,
+                "--label", test_label,
+                "--session-id", test_session_id,
+                "--task-id", test_task_id,
+                "--config-filename", test_config_filename,
+                "--config-filepath", test_config_filepath,
+            ])
+            _ = self.snapshot.execute()
+        except MutuallyExclusiveArguments:
+            exception_thrown = True
+        assert exception_thrown
+
+        # Stats exception
+        exception_thrown = False
+        try:
+            self.snapshot.parse([
+                "snapshot",
+                "create",
+                "--message", test_message,
+                "--label", test_label,
+                "--session-id", test_session_id,
+                "--task-id", test_task_id,
+                "--stats-filename", test_stats_filename,
+                "--stats-filepath", test_stats_filepath,
+            ])
+            _ = self.snapshot.execute()
+        except MutuallyExclusiveArguments:
+            exception_thrown = True
+        assert exception_thrown
+
+
+    def test_datmo_snapshot_create_default(self):
+        self.__set_variables()
         self.snapshot.parse([
             "snapshot",
             "create",

datmo/core/util/exceptions.py

@@ -88,7 +88,10 @@ class TaskRunException(Exception):
 class DatmoFolderInWorkTree(Exception):
     pass
 
-class InvalidArgumentType(ArgumentException):
+class InvalidArgumentType(Exception):
+    pass
+
+class MutuallyExclusiveArguments(Exception):
     pass
 
 class TaskNotComplete(ArgumentException):

datmo/core/util/lang/en.py

@@ -27,6 +27,7 @@
         "cli.task.run": "Error while running the task: %s",
         "cli.task.stop": "Error while stopping the task: %s",
         "util.misc_functions.get_filehash": "Filepath does not point to a valid file: %s",
+        "util.misc_functions.mutually_exclusive": "Mutually exclusive arguments passed: %s",
         "controller.code.driver.git.__init__.dne": "File path does not exist: %s",
         "controller.code.driver.git.__init__.giterror": "Error in git: %s",
         "controller.code.driver.git.__init__.gitversion": "Git version must be later than 1.9.7. Current version: %s",

datmo/core/util/misc_functions.py

@@ -11,7 +11,8 @@
 from glob import glob
 
 from datmo.core.util.i18n import get as __
-from datmo.core.util.exceptions import PathDoesNotExist
+from datmo.core.util.exceptions import PathDoesNotExist, \
+    MutuallyExclusiveArguments
 
 
 def printable_dict(input_dictionary):
@@ -85,3 +86,15 @@ def create_unique_hash(base_hash=None, salt=None):
                           datetime.datetime(1970, 1, 1)).total_seconds() * 100000
     sha1.update(salt+str(timestamp_microsec).encode('utf-8'))
     return sha1.hexdigest()
+
+def mutually_exclusive(mutually_exclusive_args, arguments_dictionary, dictionary):
+    mutually_exclusive_arg_count = 0
+    for arg in mutually_exclusive_args:
+        if arg in arguments_dictionary and arguments_dictionary[arg] is not None:
+            dictionary[arg] = arguments_dictionary[arg]
+            mutually_exclusive_arg_count+=1
+    if mutually_exclusive_arg_count>1:
+        raise MutuallyExclusiveArguments(__("error",
+                                            "util.misc_functions.mutually_exclusive",
+                                            ' '.join(mutually_exclusive_args)))
+    return

datmo/core/util/test/test_misc_functions.py

@@ -11,7 +11,8 @@
     to_unicode = str
 
 from datmo.core.util.misc_functions import get_filehash, \
-    create_unique_hash
+    create_unique_hash, mutually_exclusive
+from datmo.core.util.exceptions import MutuallyExclusiveArguments
 
 
 class TestMiscFunctions():
@@ -34,4 +35,22 @@ def test_create_unique_hash(self):
         result_hash_1 = create_unique_hash()
         result_hash_2 = create_unique_hash()
 
-        assert result_hash_1 != result_hash_2
+        assert result_hash_1 != result_hash_2
+
+    def test_mutually_exclusive(self):
+        mutually_exclusive_args = ["code_id", "commit_id"]
+        arguments_dictionary = {'code_id': 'test_code_id', 'environment_id': 'test_environment_id'}
+        dictionary = {}
+        mutually_exclusive(mutually_exclusive_args, arguments_dictionary, dictionary)
+        assert dictionary
+        assert dictionary['code_id'] == arguments_dictionary['code_id']
+
+        update_dictionary_failed = False
+        try:
+            mutually_exclusive_args = ["code_id", "commit_id"]
+            arguments_dictionary = {'code_id': 'test_code_id', 'commit_id': 'test_environment_id'}
+            dictionary = {}
+            mutually_exclusive(mutually_exclusive_args, arguments_dictionary, dictionary)
+        except MutuallyExclusiveArguments:
+            update_dictionary_failed = True
+        assert update_dictionary_failed