fix: make changes to the interactive builders operator section (#154)

* fixed interactive deployment * lint issues * fixed registry issues * lint issues
bentoml · May 16, 2022 · 161035d · 161035d
1 parent 0587ea7
commit 161035d
Show file tree

Hide file tree

Showing 6 changed files with 24 additions and 54 deletions.
diff --git a/bentoctl/cli/interactive.py b/bentoctl/cli/interactive.py
@@ -146,7 +146,7 @@ def prompt_input(
     #  prompt will display `: your_input`
     input_value = None
     if rule.get("type") == "list":
-        intended_print(f"{field_name}:", indent_level)
+        indented_print(f"{field_name}:", indent_level)
         input_value = []
         if rule.get("required") is not True:
             should_add_item_to_list = prompt_confirmation(
@@ -167,7 +167,7 @@ def prompt_input(
     elif rule.get("type") == "dict":
         input_value = {}
         if not belongs_to_list:
-            intended_print(f"{field_name}:", indent_level)
+            indented_print(f"{field_name}:", indent_level)
             for key in rule.get("schema").keys():
                 input_value[key] = prompt_input(
                     key, rule.get("schema").get(key), indent_level + 1
@@ -194,11 +194,11 @@ def prompt_input(
                 display_input_result = f"  {field_name}: {input_value}"
         else:
             display_input_result = f"{field_name}: {input_value}"
-        intended_print(display_input_result, indent_level=indent_level)
+        indented_print(display_input_result, indent_level=indent_level)
     return input_value
 
 
-def intended_print(message, indent_level=0):
+def indented_print(message, indent_level=0):
     """
     print the message with indentation
     """
@@ -258,8 +258,9 @@ def deployment_config_builder():
         if len(available_operators) == 1
         else dropdown_select("operator", available_operators)
     )
-    deployment_config["operator"] = operator_name
-    console.print(f"[b]operator:[/] {operator_name}")
+    indented_print("[b]operator:[/]")
+    indented_print(f"[b]name:[/] {operator_name}", indent_level=1)
+    deployment_config["operator"] = {"name": operator_name}
     operator = local_operator_registry.get(operator_name)
 
     # get template_type

diff --git a/bentoctl/deployment_config.py b/bentoctl/deployment_config.py
@@ -36,9 +36,14 @@ def operator_exists(field, operator_name, error):
 deployment_config_schema = {
     "name": {"required": True, "help_message": "The name for the deployment"},
     "operator": {
+        "type": "dict",
         "required": True,
-        "help_message": "The operator to use for deployment",
-        "check_with": operator_exists,
+        "schema": {
+            "name": {
+                "help_message": "The operator to use for deployment",
+                "check_with": operator_exists,
+            }
+        },
     },
     "template": {
         "required": True,

diff --git a/bentoctl/operator/registry.py b/bentoctl/operator/registry.py
@@ -47,7 +47,7 @@ def get(self, name: str):
         metadata = self.operators_list[name]
         op_path = metadata["path"]
         metadata["version"] = (
-            get_semver_version(metadata["version"]) if metadata["version"] else None
+            get_semver_version(metadata["version"]) if metadata.get("version") else None
         )
         return Operator(op_path, metadata)
 

diff --git a/tests/unit/cli/test_interactive.py b/tests/unit/cli/test_interactive.py
@@ -35,7 +35,7 @@ def test_dropdown_select(mock_terminal_menu):
 def test_inteneded_print(indent_level):
     f = StringIO()
     with redirect_stdout(f):
-        interactive_cli.intended_print("test_message", indent_level=indent_level)
+        interactive_cli.indented_print("test_message", indent_level=indent_level)
         assert f.getvalue() == f"{'    '*indent_level}test_message\n"
 
 

diff --git a/tests/unit/operator/test_operator_registry.py b/tests/unit/operator/test_operator_registry.py
@@ -18,11 +18,10 @@
 @pytest.fixture
 def op_reg(tmp_path):
     os.environ["BENTOCTL_HOME"] = str(tmp_path)
-    op_reg = registry.OperatorRegistry(tmp_path / "operators")
+    op_reg = registry.OperatorRegistry(tmp_path)
     yield op_reg
 
 
-# fix me
 def test_registry_get(op_reg):
     op_reg.install_operator(TESTOP_PATH)
     testop = op_reg.get("testop")
@@ -40,7 +39,6 @@ def test_registry_init_and_list(tmp_path):
     assert op_reg.list() == sample_op_list_json
 
 
-# fix me
 def test_registry_install_local_operator(op_reg):
     op_reg_path = op_reg.path
     assert not (op_reg_path / TEST_OPERATOR.name).exists()
@@ -57,41 +55,6 @@ def test_registry_install_local_operator(op_reg):
         op_reg.install_operator(TESTOP_PATH)
 
 
-# fix me
-@pytest.mark.parametrize(
-    "user_input, git_url, git_branch",
-    [
-        ("aws-lambda", "https://github.com/bentoml/aws-lambda-deploy.git", "main"),
-        ("bentoml/heroku", "https://github.com/bentoml/heroku.git", None),
-        (
-            "owner/heroku:branch",
-            "https://github.com/owner/heroku.git",
-            "branch",
-        ),
-        (
-            "https://github.com/bentoml/aws-sagemaker-deploy.git",
-            "https://github.com/bentoml/aws-sagemaker-deploy.git",
-            None,
-        ),
-    ],
-)
-def test_registry_install(user_input, git_url, git_branch, op_reg, monkeypatch):
-    def patched_clone_git_repo(git_url, branch=None):  # pylint: disable=W0613
-        return TESTOP_PATH
-
-    monkeypatch.setattr(registry, "_clone_git_repo", patched_clone_git_repo)
-
-    op_reg.install_operator(user_input)
-    assert (op_reg.path / TEST_OPERATOR.name).exists()
-    assert op_reg.operators_list[TEST_OPERATOR.name] == {
-        "path": str(op_reg.path / TEST_OPERATOR.name),
-        "git_url": git_url,
-        "git_branch": git_branch,
-        "is_local": False,
-    }
-
-
-# fix me
 def test_registry_update_local_operator(op_reg, tmpdir):
     tmp_testop_path = os.path.join(tmpdir, "testop")
     shutil.copytree(TESTOP_PATH, tmp_testop_path)
@@ -107,7 +70,6 @@ def test_registry_update_local_operator(op_reg, tmpdir):
     assert path_to_first_file_before_updation == path_to_first_file_after_updation
 
 
-# fix me
 def test_registry_remove(op_reg):
     op_reg.install_operator(TESTOP_PATH)
     testop_path = op_reg.path / "testop"

diff --git a/tests/unit/test_deployment_config.py b/tests/unit/test_deployment_config.py
@@ -46,7 +46,7 @@ def test_deployment_config_init(get_mock_operator_registry, monkeypatch):
                 "api_version": "v1",
                 "name": "test",
                 "template": "terraform",
-                "operator": "testop",
+                "operator": {"name": "testop"},
                 "spec": {},
             }
         )
@@ -58,7 +58,7 @@ def test_deployment_config_init(get_mock_operator_registry, monkeypatch):
                 "api_version": "v1",
                 "name": "test",
                 "template": "",
-                "operator": "testop",
+                "operator": {"name": "testop"},
                 "spec": {},
             }
         )
@@ -70,7 +70,7 @@ def test_deployment_config_init(get_mock_operator_registry, monkeypatch):
                 "api_version": "v1",
                 "name": "test",
                 "template": "not-valid-template",
-                "operator": "testop",
+                "operator": {"name": "testop"},
                 "spec": {},
             }
         )
@@ -79,7 +79,8 @@ def test_deployment_config_init(get_mock_operator_registry, monkeypatch):
 VALID_YAML = """
 api_version: v1
 name: test
-operator: testop
+operator:
+    name: testop
 template: terraform
 spec:
     project_id: testproject
@@ -93,7 +94,8 @@ def test_deployment_config_init(get_mock_operator_registry, monkeypatch):
 VALID_YAML_INVALID_SCHEMA = """
 api_version: v1
 name: test
-operator: testop
+operator:
+    name: testop
 template: terraform
 spec:
     project_id: testproject