Merge branch '65_add_push_dependency_failure_check' into 'master'

Add push failure check

Closes #65

See merge request !88

beat/cmdline/algorithms.py

@@ -45,7 +45,6 @@ from beat.core.execution import DockerExecutor
 from beat.core.dock import Host
 from beat.core import hash
 from beat.backend.python.algorithm import Storage as AlgorithmStorage
-from beat.backend.python.algorithm import Algorithm
 
 from . import common
 from . import commands
@@ -330,9 +329,7 @@ def execute_impl(prefix, cache, instructions_file):
 
 def get_dependencies(ctx, asset_name):
     prefix = ctx.meta["config"].path
-    alg = Algorithm(prefix, asset_name)
-    if not alg.valid:
-        raise RuntimeError("Invalid algortihm: {}".format(alg.errors))
+    alg = algorithm.Algorithm(prefix, asset_name)
 
     dependencies = {}
 

beat/cmdline/commands.py

@@ -282,6 +282,12 @@ def push_impl(ctx, names, force, dry_run):
     mappings = ctx.meta.get("mappings", {})
 
     for name in names:
+        validator = common.TYPE_VALIDATOR[asset_info.asset_type](config.path, name)
+        if not validator.valid:
+            raise RuntimeError(
+                "Invalid {} {}".format(asset_info.asset_type, validator.errors)
+            )
+
         with common.Selector(config.path) as selector:
             dependency_type = common.TYPE_PLURAL[asset_info.asset_type]
             fork = selector.forked_from(asset_info.asset_type, name)

beat/cmdline/dataformats.py

@@ -39,8 +39,6 @@ import click
 
 from beat.core import dataformat
 
-from beat.backend.python.dataformat import DataFormat
-
 from . import common
 from . import commands
 
@@ -135,9 +133,7 @@ def pull_impl(webapi, prefix, names, force, indentation, cache):
 
 def get_dependencies(ctx, asset_name):
     prefix = ctx.meta["config"].path
-    df = DataFormat(prefix, asset_name)
-    if not df.valid:
-        raise RuntimeError("Invalid dataformat: {}".format(df.errors))
+    df = dataformat.DataFormat(prefix, asset_name)
 
     dependencies = {}
 

beat/cmdline/experiments.py

@@ -714,8 +714,6 @@ def plot_impl(
 def get_dependencies(ctx, asset_name):
     prefix = ctx.meta["config"].path
     exp = Experiment(prefix, asset_name)
-    if not exp.valid:
-        raise RuntimeError("Invalid experiment {}".format(exp.errors))
 
     dependencies = {"toolchains": [exp.toolchain.name]}
 

beat/cmdline/libraries.py

@@ -78,8 +78,6 @@ import click
 
 from beat.core import library
 
-from beat.backend.python.library import Library
-
 from . import common
 from . import commands
 
@@ -170,9 +168,7 @@ def pull_impl(webapi, prefix, names, force, indentation, cache):
 
 def get_dependencies(ctx, asset_name):
     prefix = ctx.meta["config"].path
-    lib = Library(prefix, asset_name)
-    if not lib.valid:
-        raise RuntimeError("Invalid library: {}".format(lib.errors))
+    lib = library.Library(prefix, asset_name)
 
     dependencies = {}
 

beat/cmdline/test/core.py

@@ -147,6 +147,7 @@ class BaseTest:
         use_platform = kwargs.get("platform", platform)
         use_cache = kwargs.get("cache", "cache")
         asset_type = kwargs.get("asset_type", cls.asset_type)
+        remote_user = kwargs.get("user", user)
 
         cmd_group = cls.get_cmd_group(asset_type)
 
@@ -157,7 +158,7 @@ class BaseTest:
             "--token",
             token,
             "--user",
-            user,
+            remote_user,
             "--cache",
             use_cache,
             "--platform",
@@ -432,6 +433,17 @@ class AssetRemoteTest(AssetBaseTest):
         exit_code, output = self.call("rm", "--remote", asset_name)
         nose.tools.eq_(exit_code, 0, output)
 
+    @slow
+    @skip_disconnected
+    def test_fail_push_invalid(self):
+        asset_name = self.object_map["push_invalid"]
+
+        with nose.tools.assert_raises(RuntimeError) as assertion:
+            self.call("push", asset_name, user="errors")
+        exc = assertion.exception
+        text = exc.args[0]
+        nose.tools.assert_true(text.startswith("Invalid "))
+
     @slow
     @skip_disconnected
     def test_fail_not_owner_push(self):

beat/cmdline/test/prefix/experiments/other_user/other_user/somechain/1/someexp.json → beat/cmdline/test/prefix/experiments/other_user/user/single/1/someexp.json

 {
     "schema_version": 2,
     "blocks": {
-        "some_block": {
-            "algorithm": "user/unknown/1",
+        "echo": {
+            "algorithm": "v1/integers_echo/1",
             "inputs": {
-                "in1": "in1"
+                "in_data": "in"
             },
             "outputs": {
-                "out": "out"
+                "out_data": "out"
             }
         }
     },
@@ -15,14 +15,14 @@
     },
     "analyzers": {
         "analysis": {
-            "algorithm": "user/integers_echo_analyzer_v2/1",
+            "algorithm": "v1/integers_echo_analyzer/1",
             "inputs": {
-                "in_data": "input"
+                "in_data": "in"
             }
         }
     },
     "datasets": {
-        "integers": {
+        "set": {
             "database": "simple/1",
             "protocol": "protocol",
             "set": "set"

beat/cmdline/test/prefix/libraries/other_user/baselib/1.json

+{
+  "language": "python"
+}

beat/cmdline/test/prefix/libraries/other_user/baselib/1.py

+#!/usr/bin/env python
+# vim: set fileencoding=utf-8 :
+
+###################################################################################
+#                                                                                 #
+# Copyright (c) 2019 Idiap Research Institute, http://www.idiap.ch/               #
+# Contact: beat.support@idiap.ch                                                  #
+#                                                                                 #
+# Redistribution and use in source and binary forms, with or without              #
+# modification, are permitted provided that the following conditions are met:     #
+#                                                                                 #
+# 1. Redistributions of source code must retain the above copyright notice, this  #
+# list of conditions and the following disclaimer.                                #
+#                                                                                 #
+# 2. Redistributions in binary form must reproduce the above copyright notice,    #
+# this list of conditions and the following disclaimer in the documentation       #
+# and/or other materials provided with the distribution.                          #
+#                                                                                 #
+# 3. Neither the name of the copyright holder nor the names of its contributors   #
+# may be used to endorse or promote products derived from this software without   #
+# specific prior written permission.                                              #
+#                                                                                 #
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND #
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED   #
+# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE          #
+# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE    #
+# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL      #
+# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR      #
+# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER      #
+# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,   #
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE   #
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.            #
+#                                                                                 #
+###################################################################################
+
+
+import sys
+
+
+def f():
+    return "OK"
+
+
+def pyver():
+    return "%d.%d" % sys.version_info[:2]

beat/cmdline/test/prefix/libraries/other_user/baselib/1.rst

+Test documentation

beat/cmdline/test/test_algorithms.py

@@ -59,6 +59,7 @@ class TestOnline(core.OnlineAssetTestCase):
         "fork": "user/forked_obj/1",
         "push": "user/db_input_loop_processor/1",
         "not_owner_push": "v1/integers_add/1",
+        "push_invalid": "errors/description_too_long/1",
     }
 
     def _modify_asset(self, asset_name):

beat/cmdline/test/test_databases.py

@@ -120,6 +120,10 @@ class TestOnline(core.OnlineAssetTestCase):
     def test_push_different_forks(self):
         raise nose.SkipTest("Databases don't allow push")
 
+    @core.skip_disconnected
+    def test_fail_push_invalid(self):
+        raise nose.SkipTest("Databases don't allow push")
+
     @core.skip_disconnected
     def test_fail_not_owner_push(self):
         """No owner so not need to test"""

beat/cmdline/test/test_dataformats.py

@@ -54,6 +54,7 @@ class TestOnline(core.OnlineAssetTestCase):
         "fork": "user/forked_obj/1",
         "push": "user/composed/1",
         "not_owner_push": "system/chart/1",
+        "push_invalid": "errors/description_too_long/1",
     }
 
     def _modify_asset(self, asset_name):

beat/cmdline/test/test_experiments.py

@@ -48,7 +48,6 @@ from beat.core.experiment import Storage, Experiment
 from beat.core.toolchain import Storage as TCStorage
 from beat.core.algorithm import Storage as AlgStorage
 from beat.core.dataformat import Storage as DFStorage
-from beat.core.database import Storage as DBStorage
 
 from .utils import index_experiment_dbs, MockLoggingHandler
 
@@ -93,7 +92,8 @@ class TestOnline(core.OnlineAssetTestCase):
         "push": "user/user/unknown/1/unknown",
         "fork_from": "user/user/unknown/1/unknown",
         "fork": "user/user/unknown/1/forked_obj",
-        "not_owner_push": "other_user/other_user/somechain/1/someexp",
+        "not_owner_push": "other_user/user/single/1/someexp",
+        "push_invalid": "errors/user/single/1/description_too_long",
     }
 
     def _modify_asset(self, asset_name):
@@ -106,7 +106,6 @@ class TestOnline(core.OnlineAssetTestCase):
     def _prepare_fork_dependencies(self, asset_name):
         super()._prepare_fork_dependencies(asset_name)
         assets = [
-            (DBStorage, ["simple/1"]),
             (DFStorage, ["user/single_integer/1"]),
             (AlgStorage, ["user/unknown/1", "user/integers_echo_analyzer_v2/1"]),
             (TCStorage, ["user/unknown/1"]),
@@ -120,6 +119,21 @@ class TestOnline(core.OnlineAssetTestCase):
                 dst_storage = storage_cls(tmp_prefix, asset)
                 dst_storage.save(*src_storage.load())
 
+        # The remote database dataformats is not the same as the local version
+        # hence download it to ensure we can push experiments using it properly
+        self.call("pull", "simple/1", asset_type="databases", prefix=tmp_prefix)
+
+    @slow
+    @core.skip_disconnected
+    def test_push_and_delete(self):
+        asset_name = self.object_map["push"]
+        self._prepare_fork_dependencies(asset_name)
+
+        exit_code, output = self.call("push", asset_name, prefix=tmp_prefix)
+        nose.tools.eq_(exit_code, 0, output)
+        exit_code, output = self.call("rm", "--remote", asset_name)
+        nose.tools.eq_(exit_code, 0, output)
+
     @core.skip_disconnected
     def test_draw(self):
         obj = "user/user/double_triangle/1/double_triangle"

beat/cmdline/test/test_libraries.py

@@ -66,7 +66,8 @@ class TestOnline(core.OnlineAssetTestCase):
         "fork_from": "user/nest1/1",
         "fork": "user/forked_obj/1",
         "push": "user/nest1/1",
-        "not_owner_push": "errors/invalid_mix/1",
+        "not_owner_push": "other_user/baselib/1",
+        "push_invalid": "errors/duplicate_key/1",
     }
 
     def _modify_asset(self, asset_name):

beat/cmdline/test/test_plotterparameters.py

@@ -83,6 +83,10 @@ class TestOnline(core.OnlineAssetTestCase):
     def test_push_different_forks(self):
         raise nose.SkipTest("Plotterparameters don't allow push")
 
+    @core.skip_disconnected
+    def test_fail_push_invalid(self):
+        raise nose.SkipTest("Plotterparameters don't allow push")
+
     @core.skip_disconnected
     def test_fail_not_owner_push(self):
         """No owner so not need to test"""

beat/cmdline/test/test_plotters.py

@@ -86,6 +86,10 @@ class TestOnline(core.OnlineAssetTestCase):
     def test_push_different_forks(self):
         raise nose.SkipTest("Plotters don't allow push")
 
+    @core.skip_disconnected
+    def test_fail_push_invalid(self):
+        raise nose.SkipTest("Plotters don't allow push")
+
     @core.skip_disconnected
     def test_fail_not_owner_push(self):
         """No owner so not need to test"""

beat/cmdline/test/test_toolchains.py

@@ -57,6 +57,7 @@ class TestOnline(core.OnlineAssetTestCase):
         "fork": "user/forked_obj/1",
         "push": "user/two_loops/1",
         "not_owner_push": "other_user/somechain/1",
+        "push_invalid": "errors/invalid/1",
     }
 
     def _modify_asset(self, asset_name):