View install commands (#596)

* Removed the `--view` option of `shpc install`

as per #590 (comment)

* bugfix: always install in the default view, alongside additionally requested views
Renamed the variables to make it clear what is a view and what is a view name.

* Print what bash is running
* Test views on tcsh too
* Moved the code to install a module into a view to a separate function
* addition of intermediate module class

This will help to carry around some common data
attributes and functions, and reduce redundancy within
the ModuleBase class. It also allows for providing
fewer variables to the rendered templates since
they can come from the module.

* ensure we maintain parsed name
* Moved the view uninstallation to a separate function to mirror view_install
* No need to make this local variable
* Use parsed_name for consistency
* Document the "force" flags

Signed-off-by: vsoch <[email protected]>
Co-authored-by: vsoch <[email protected]>

Author: muffato

.github/workflows/test.yml

@@ -98,6 +98,7 @@ jobs:
           printf "\n\nmodule help ============================================\n"
           module help python/3.9.5-alpine
 
+          set -x
           python-exec echo donuts >test_output
           cat test_output
           grep --quiet donuts test_output
@@ -109,13 +110,14 @@ jobs:
           cat test_output
           grep --quiet 'Python 3.9.5' test_output
           rm test_output
-          shpc uninstall --force python:3.9.5-alpine
 
-          # Try creating views install
           mkdir -p tmp-modules
           shpc config set views_base:tmp-modules
           shpc view create noodles
-          shpc install --view noodles python:3.9.5-alpine
+          shpc view install noodles python:3.9.5-alpine
+
+          shpc uninstall --force python:3.9.5-alpine
+          shpc view --force delete noodles
  
       - name: Run python module tests (tcsh)
         shell: tcsh -e {0}
@@ -164,4 +166,11 @@ jobs:
           cat test_output
           grep --quiet 'Python 3.9.5' test_output
           rm test_output
+
+          mkdir -p tmp-modules
+          shpc config set views_base:tmp-modules
+          shpc view create noodles
+          shpc view install noodles python:3.9.5-alpine
+
           shpc uninstall --force python:3.9.5-alpine
+          shpc view --force delete noodles

shpc/client/__init__.py

@@ -101,12 +101,6 @@ def get_parser():
         "install_recipe",
         help="recipe to install\nshpc install python\nshpc install python:3.9.5-alpine",
     )
-    install.add_argument(
-        "--view",
-        dest="view",
-        help="install module to a named view (must be installed to shpc first).",
-        default=None,
-    )
     install.add_argument(
         "--no-view",
         dest="no_view",

shpc/client/install.py

@@ -3,7 +3,6 @@
 __license__ = "MPL 2.0"
 
 import shpc.utils
-from shpc.logger import logger
 
 
 def main(args, parser, extra, subparser):
@@ -25,11 +24,9 @@ def main(args, parser, extra, subparser):
     # Update config settings on the fly
     cli.settings.update_params(args.config_params)
 
-    # It doesn't make sense to define view and no view
-    if args.view and args.no_view:
-        logger.exit("Conflicting arguments --view and --no-view, choose one.")
-
     # And do the install
-    cli.install(
-        args.install_recipe, view=args.view, disable_view=args.no_view, force=args.force
-    )
+    cli.install(args.install_recipe, force=args.force)
+    if cli.settings.default_view and not args.no_view:
+        cli.view_install(
+            cli.settings.default_view, args.install_recipe, force=args.force
+        )

shpc/client/view.py

@@ -52,7 +52,10 @@ def create_from_file(
 
     # Extra modules to install
     for install_module in install_modules:
-        cli.install(install_module, view=view_name, disable_view=False, force=force)
+
+        # TODO: can we cut out early if already installed?
+        cli.install(install_module, force=force)
+        cli.view_install(view_name, install_module, force=force)
 
 
 def main(args, parser, extra, subparser):
@@ -168,7 +171,8 @@ def main(args, parser, extra, subparser):
     # We don't make it hard to require them to install to the root first
     module_name = args.params.pop(0)
     if command == "install":
-        cli.install(module_name, view=view_name, disable_view=False, force=args.force)
+        cli.install(module_name, force=args.force)
+        cli.view_install(view_name, module_name, force=args.force)
 
     if command == "uninstall":
-        cli.uninstall(module_name, view=view_name, force=args.force)
+        cli.view_uninstall(view_name, module_name, force=args.force)

shpc/main/__init__.py

@@ -41,17 +41,17 @@ def get_client(quiet=False, **kwargs):
 
     # Add the container operator
     if container == "singularity":
-        from .container import SingularityContainer
+        from shpc.main.container import SingularityContainer
 
         Client.container = SingularityContainer()
 
     elif container == "podman":
-        from .container import PodmanContainer
+        from shpc.main.container import PodmanContainer
 
         Client.container = PodmanContainer()
 
     elif container == "docker":
-        from .container import DockerContainer
+        from shpc.main.container import DockerContainer
 
         Client.container = DockerContainer()
 
@@ -64,6 +64,8 @@ def get_client(quiet=False, **kwargs):
         logger.warning(
             "%s is not installed, functionality might be limited." % container.upper()
         )
+
+    # Pass on settings and container to module too
     Client.quiet = quiet
     Client.settings = settings
     return Client()

shpc/main/container/docker.py

@@ -185,21 +185,7 @@ def test_script(self, image, test_script):
         # Return code
         return result["return_code"]
 
-    def install(
-        self,
-        module_path,
-        container_path,
-        name,
-        template,
-        parsed_name,
-        aliases=None,
-        url=None,
-        description=None,
-        version=None,
-        config_features=None,
-        features=None,
-        config=None,
-    ):
+    def install(self, module_path, template, module, features=None):
         """Install a general container path to a module
 
         The module_dir should be created by the calling function, and
@@ -210,50 +196,45 @@ def install(
         # Container features are defined in container.yaml and the settings
         # and specific values are determined by the container technology
         features = self.get_features(
-            config_features, self.settings.container_features, features
+            module.config.features, self.settings.container_features, features
         )
 
         # Ensure that the container exists
         # Do we want to clean up other versions here too?
-        manifest = self.inspect(container_path)
+        manifest = self.inspect(module.container_path)
         if not manifest:
-            sys.exit("Container %s was not found. Was it pulled?" % container_path)
+            sys.exit(
+                "Container %s was not found. Was it pulled?" % module.container_path
+            )
 
         labels = manifest[0].get("Labels", {})
 
-        # If there's a tag in the name, don't use it
-        name = name.split(":", 1)[0]
-
         # Option to create wrapper scripts for commands
-        module_dir = os.path.dirname(module_path)
+        aliases = module.config.get_aliases()
         wrapper_scripts = []
 
         # Wrapper scripts can be global (for aliases) or container specific
         if self.settings.wrapper_scripts["enabled"] is True:
             wrapper_scripts = shpc.main.wrappers.generate(
                 aliases=aliases,
-                module_dir=module_dir,
+                module_dir=module.module_dir,
                 features=features,
                 container=self,
-                image=container_path,
-                config=config,
+                image=module.container_path,
+                config=module.config,
             )
 
         # Make sure to render all values!
         out = template.render(
             settings=self.settings,
             shell=self.shell_path,
-            image=container_path,
-            description=description,
             aliases=aliases,
-            url=url,
             features=features,
-            version=version,
             labels=labels,
             creation_date=datetime.now(),
-            name=name,
-            parsed_name=parsed_name,
             command=self.command,
+            module=module,
+            parsed_name=module.config.name,
             wrapper_scripts=wrapper_scripts,
         )
         shpc.utils.write_file(module_path, out)

shpc/main/container/singularity.py

@@ -165,22 +165,7 @@ def _add_docker_image(self, name, tag, image, config, container_yaml, **kwargs):
         config.add_tag(tag, tags[tag])
         return config
 
-    def install(
-        self,
-        module_path,
-        container_path,
-        name,
-        template,
-        parsed_name,
-        aliases=None,
-        template_name=None,
-        url=None,
-        description=None,
-        config_features=None,
-        features=None,
-        version=None,
-        config=None,
-    ):
+    def install(self, module_path, template, module, features=None):
         """Install a general container path to a module
 
         The module_dir should be created by the calling function, and
@@ -191,19 +176,19 @@ def install(
         # Container features are defined in container.yaml and the settings
         # and specific values are determined by the container technology
         features = self.get_features(
-            config_features, self.settings.container_features, features
+            module.config.features, self.settings.container_features, features
         )
 
         # Remove any previous containers
-        container_dir = os.path.dirname(container_path)
+        container_dir = os.path.dirname(module.container_path)
         for older in glob("%s%s*.sif" % (container_dir, os.sep)):
-            if older == container_path:
+            if older == module.container_path:
                 continue
             os.remove(older)
 
         # Get inspect metadata from the container (only if singularity installed
         try:
-            metadata = self.inspect(container_path)
+            metadata = self.inspect(module.container_path)
 
             # Add labels, and deffile
             labels = metadata.get("attributes", {}).get("labels")
@@ -216,34 +201,30 @@ def install(
             labels = {}
 
         # Option to create wrapper scripts for commands
-        module_dir = os.path.dirname(module_path)
+        aliases = module.config.get_aliases()
 
         # Wrapper scripts can be global (for aliases) or container specific
         wrapper_scripts = []
         if self.settings.wrapper_scripts["enabled"] is True:
             wrapper_scripts = shpc.main.wrappers.generate(
                 aliases=aliases,
-                module_dir=module_dir,
+                module_dir=module.module_dir,
                 features=features,
                 container=self,
-                image=container_path,
-                config=config,
+                image=module.container_path,
+                config=module.config,
             )
 
         # Make sure to render all values!
         out = template.render(
             settings=self.settings,
-            container_sif=container_path,
-            description=description,
             aliases=aliases,
-            url=url,
             features=features,
-            version=version,
             labels=labels,
             deffile=deffile,
             creation_date=datetime.now(),
-            name=name,
-            parsed_name=parsed_name,
+            module=module,
+            parsed_name=module.config.name,
             wrapper_scripts=wrapper_scripts,
         )
         utils.write_file(module_path, out)