fix: implement feedback

eduNEXT · Aug 20, 2024 · 3a750dc · 3a750dc
1 parent e55f4b9
commit 3a750dc
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 60 deletions.
diff --git a/tutorpicasso/commands/enable_private_packages.py b/tutorpicasso/commands/enable_private_packages.py
@@ -17,7 +17,7 @@ def enable_private_packages() -> None:
     defining them as private.
 
     Raises:
-        Exception: If an error occurs during the cloning or defining process.
+        Exception: If there is not enough information to clone the repo.
     """
     context = click.get_current_context().obj
     tutor_root = context.root
@@ -41,39 +41,33 @@ def enable_private_packages() -> None:
             file.write("")
 
     for package, info in packages.items():
-        try:
-            if not {"name", "repo", "version"}.issubset(info):
-                raise KeyError(
-                    f"{package} is missing one of the required keys: 'name', 'repo', 'version'"
-                )
-
-            if os.path.isdir(f'{private_requirements_root}/{info["name"]}'):
-                subprocess.call(
-                    ["rm", "-rf", f'{private_requirements_root}/{info["name"]}']
-                )
+        if not {"name", "repo", "version"}.issubset(info):
+            raise click.ClickException(
+                f"{package} is missing one of the required keys: 'name', 'repo', 'version'"
+            )
 
+        if os.path.isdir(f'{private_requirements_root}/{info["name"]}'):
             subprocess.call(
-                ["git", "clone", "-b", info["version"], info["repo"]],
-                cwd=private_requirements_root,
+                ["rm", "-rf", f'{private_requirements_root}/{info["name"]}']
             )
 
-            if tutor_version_obj < quince_version_obj:
-                echo_command = (
-                    f'echo "-e ./{info["name"]}/" >> {private_requirements_txt}'
-                )
-                subprocess.call(echo_command, shell=True)
-            else:
-                subprocess.call(
-                    [
-                        "tutor",
-                        "mounts",
-                        "add",
-                        f'{private_requirements_root}/{info["name"]}',
-                    ]
-                )
+        subprocess.call(
+            ["git", "clone", "-b", info["version"], info["repo"]],
+            cwd=private_requirements_root,
+        )
 
-        except KeyError as e:
-            raise click.ClickException(str(e))
+        if tutor_version_obj < quince_version_obj:
+            echo_command = f'echo "-e ./{info["name"]}/" >> {private_requirements_txt}'
+            subprocess.call(echo_command, shell=True)
+        else:
+            subprocess.call(
+                [
+                    "tutor",
+                    "mounts",
+                    "add",
+                    f'{private_requirements_root}/{info["name"]}',
+                ]
+            )
 
 
 def get_picasso_packages(settings: Dict[str, Any]) -> Dict[str, Dict[str, Any]]:
@@ -88,8 +82,6 @@ def get_picasso_packages(settings: Dict[str, Any]) -> Dict[str, Dict[str, Any]]:
         and the values are package details.
     """
     picasso_packages = {
-        key: val
-        for key, val in settings.items()
-        if key.endswith("_DPKG") and val != "None"
+        key: val for key, val in settings.items() if key.endswith("_DPKG") and val
     }
     return picasso_packages
diff --git a/tutorpicasso/commands/enable_themes.py b/tutorpicasso/commands/enable_themes.py
@@ -18,21 +18,20 @@ def enable_themes() -> None:
     tutor_root = context.root
     tutor_conf = tutor_config.load(tutor_root)
 
-    if tutor_conf.get("PICASSO_THEMES"):
-        for theme in tutor_conf["PICASSO_THEMES"]:
-            try:
-                if not {"name", "repo", "version"}.issubset(theme.keys()):
-                    raise KeyError(
-                        f"{theme} is missing one or more required keys: "
-                        "'name', 'repo', 'version'"
-                    )
-
-                theme_path = f'{tutor_root}/env/build/openedx/themes/{theme["name"]}'
-                if os.path.isdir(theme_path):
-                    subprocess.call(["rm", "-rf", theme_path])
-
-                subprocess.call(
-                    ["git", "clone", "-b", theme["version"], theme["repo"], theme_path],
-                )
-            except KeyError as e:
-                raise click.ClickException(f"Error: {str(e)}")
+    if not tutor_conf.get("PICASSO_THEMES"):
+        return
+
+    for theme in tutor_conf["PICASSO_THEMES"]:
+        if not {"name", "repo", "version"}.issubset(theme.keys()):
+            raise click.ClickException(
+                f"{theme} is missing one or more required keys: "
+                "'name', 'repo', 'version'"
+            )
+
+        theme_path = f'{tutor_root}/env/build/openedx/themes/{theme["name"]}'
+        if os.path.isdir(theme_path):
+            subprocess.call(["rm", "-rf", theme_path])
+
+        subprocess.call(
+            ["git", "clone", "-b", theme["version"], theme["repo"], theme_path],
+        )
diff --git a/tutorpicasso/commands/run_extra_commands.py b/tutorpicasso/commands/run_extra_commands.py
@@ -15,16 +15,16 @@ def run_extra_commands() -> None:
     """
     This command runs tutor commands defined in PICASSO_EXTRA_COMMANDS
     """
-    tutor_root = (
-        subprocess.check_output("tutor config printroot", shell=True)
-        .decode("utf-8")
-        .strip()
-    )
-    config: Any = tutor_config.load(tutor_root)
-    picasso_extra_commands: Any = config.get("PICASSO_EXTRA_COMMANDS", None)
-    if picasso_extra_commands is not None:
-        validate_commands(picasso_extra_commands)
-        list(map(run_command, picasso_extra_commands))
+    context = click.get_current_context().obj
+    tutor_conf = tutor_config.load(context.root)
+
+    picasso_extra_commands: Any = tutor_conf.get("PICASSO_EXTRA_COMMANDS", None)
+
+    if not picasso_extra_commands:
+        return
+
+    validate_commands(picasso_extra_commands)
+    list(map(run_command, picasso_extra_commands))
 
 
 def validate_commands(commands: Any) -> None: