Merge pull request #1605 from rhatdan/chat

Switchout hasattr for getattr wherever possible

Author: ericcurtin

.github/workflows/ci.yml

@@ -207,6 +207,10 @@ jobs:
            sudo mv /tmp/daemon.json /etc/docker/daemon.json
            cat /etc/docker/daemon.json
            sudo systemctl restart docker.service
+           sudo mkdir -m a=rwx -p /mnt/tmp /mnt/runner
+           sudo mkdir -m o=rwx -p /home/runner/.local
+           sudo chown runner:runner /mnt/runner /home/runner/.local
+           sudo mount --bind /mnt/runner /home/runner/.local
            df -h
 
       - name: Build a container for CPU inferencing

ramalama/cli.py

@@ -262,7 +262,7 @@ def parse_arguments(parser):
 
 def post_parse_setup(args):
     """Perform additional setup after parsing arguments."""
-    if hasattr(args, "MODEL") and args.subcommand != "rm":
+    if getattr(args, "MODEL", None) and args.subcommand != "rm":
         resolved_model = shortnames.resolve(args.MODEL)
         if resolved_model:
             args.UNRESOLVED_MODEL = args.MODEL

ramalama/engine.py

@@ -57,11 +57,11 @@ def add_pull_newer(self):
             self.exec_args += ["--pull", self.args.pull]
 
     def add_network(self):
-        if hasattr(self.args, "network") and self.args.network:
+        if getattr(self.args, "network", None):
             self.exec_args += ["--network", self.args.network]
 
     def add_oci_runtime(self):
-        if hasattr(self.args, "oci_runtime") and self.args.oci_runtime:
+        if getattr(self.args, "oci_runtime", None):
             self.exec_args += ["--runtime", self.args.oci_runtime]
             return
         if check_nvidia() == "cuda":
@@ -71,13 +71,13 @@ def add_oci_runtime(self):
                 self.exec_args += ["--runtime", "/usr/bin/nvidia-container-runtime"]
 
     def add_privileged_options(self):
-        if hasattr(self.args, "privileged") and self.args.privileged:
+        if getattr(self.args, "privileged", False):
             self.exec_args += ["--privileged"]
         else:
             self.exec_args += [
                 "--security-opt=label=disable",
             ]
-            if not hasattr(self.args, "nocapdrop"):
+            if not getattr(self.args, "nocapdrop", False):
                 self.exec_args += [
                     "--cap-drop=all",
                     "--security-opt=no-new-privileges",
@@ -89,12 +89,9 @@ def cap_add(self, cap):
     def use_tty(self):
         if not sys.stdin.isatty():
             return False
-        if not (hasattr(self.args, "ARGS") and self.args.ARGS):
+        if getattr(self.args, "ARGS", None):
             return False
-        if not (hasattr(self.args, "subcommand") and self.args.subcommand == "run"):
-            return False
-
-        return True
+        return getattr(self.args, "subcommand", "") != "run"
 
     def add_subcommand_env(self):
         if EMOJI and self.use_tty():
@@ -104,20 +101,19 @@ def add_subcommand_env(self):
                 self.exec_args += ["--env", "LLAMA_PROMPT_PREFIX=🐋 > "]
 
     def add_env_option(self):
-        if hasattr(self.args, "env"):
-            for env in self.args.env:
-                self.exec_args += ["--env", env]
+        for env in getattr(self.args, "env", []):
+            self.exec_args += ["--env", env]
 
     def add_tty_option(self):
         if self.use_tty():
             self.exec_args += ["-t"]
 
     def add_detach_option(self):
-        if hasattr(self.args, "detach") and self.args.detach is True:
+        if getattr(self.args, "detach", False):
             self.exec_args += ["-d"]
 
     def add_port_option(self):
-        if not hasattr(self.args, "port") or not self.args.port or self.args.port == "":
+        if getattr(self.args, "port", "") == "":
             return
 
         if self.args.port.count(":") > 0:
@@ -126,7 +122,7 @@ def add_port_option(self):
             self.exec_args += ["-p", f"{self.args.port}:{self.args.port}"]
 
     def add_device_options(self):
-        if hasattr(self.args, "device") and self.args.device:
+        if getattr(self.args, "device", None):
             for device_arg in self.args.device:
                 self.exec_args += ["--device", device_arg]
 
@@ -151,7 +147,7 @@ def add_device_options(self):
             self.exec_args += ["-e", f"{k}={v}"]
 
     def add_rag(self):
-        if not hasattr(self.args, "rag") or not self.args.rag:
+        if not getattr(self.args, "rag", None):
             return
 
         if os.path.exists(self.args.rag):
@@ -162,7 +158,7 @@ def add_rag(self):
             self.exec_args.append(f"--mount=type=image,source={self.args.rag},destination=/rag,rw=true")
 
     def handle_podman_specifics(self):
-        if hasattr(self.args, "podman_keep_groups") and self.args.podman_keep_groups:
+        if getattr(self.args, "podman_keep_groups", None):
             self.exec_args += ["--group-add", "keep-groups"]
 
     def add(self, newargs):
@@ -195,10 +191,10 @@ def images(args):
         raise ValueError("no container manager (Podman, Docker) found")
 
     conman_args = [conman, "images"]
-    if hasattr(args, "noheading") and args.noheading:
+    if getattr(args, "noheading", False):
         conman_args += ["--noheading"]
 
-    if hasattr(args, "notrunc") and args.notrunc:
+    if getattr(args, "notrunc", False):
         conman_args += ["--no-trunc"]
 
     if args.format:
@@ -220,10 +216,10 @@ def containers(args):
         raise ValueError("no container manager (Podman, Docker) found")
 
     conman_args = [conman, "ps", "-a", "--filter", "label=ai.ramalama"]
-    if hasattr(args, "noheading") and args.noheading:
+    if getattr(args, "noheading", False):
         conman_args += ["--noheading"]
 
-    if hasattr(args, "notrunc") and args.notrunc:
+    if getattr(args, "notrunc", False):
         conman_args += ["--no-trunc"]
 
     if args.format:
@@ -332,6 +328,5 @@ def add_labels(args, add_label):
         "subcommand": "ai.ramalama.command",
     }
     for arg, label_prefix in label_map.items():
-        if hasattr(args, arg):
-            if value := getattr(args, arg):
-                add_label(f"{label_prefix}={value}")
+        if value := getattr(args, arg, None):
+            add_label(f"{label_prefix}={value}")

ramalama/kube.py

@@ -7,11 +7,9 @@
 
 class Kube:
     def __init__(self, model, chat_template, args, exec_args):
-        self.ai_image = model
-        if hasattr(args, "MODEL"):
-            self.ai_image = args.MODEL
+        self.ai_image = getattr(args, "MODEL", model)
         self.ai_image = self.ai_image.removeprefix("oci://")
-        if hasattr(args, "name") and args.name:
+        if getattr(args, "name", None):
             self.name = args.name
         else:
             self.name = genname()

ramalama/model.py

@@ -197,18 +197,18 @@ def remove(self, args):
         return
 
     def get_container_name(self, args):
-        if hasattr(args, "name") and args.name:
+        if getattr(args, "name", None):
             return args.name
 
         return genname()
 
     def base(self, args, name):
         # force accel_image to use -rag version. Drop TAG if it exists
         # so that accel_image will add -rag to the image specification.
-        if args.image == self.default_image and (hasattr(args, "rag") and args.rag):
+        if args.image == self.default_image and getattr(args, "rag", None):
             args.image = rag_image(args.image)
         self.engine = Engine(args)
-        if args.subcommand == "run" and not (hasattr(args, "ARGS") and args.ARGS) and sys.stdin.isatty():
+        if args.subcommand == "run" and not getattr(args, "ARGS", None) and sys.stdin.isatty():
             self.engine.add(["-i"])
 
         self.engine.add(
@@ -236,7 +236,7 @@ def add_oci_runtime(self, conman_args, args):
         return conman_args
 
     def add_rag(self, exec_args, args):
-        if not hasattr(args, "rag") or not args.rag:
+        if not getattr(args, "rag", None):
             return exec_args
 
         if os.path.exists(args.rag):
@@ -290,8 +290,8 @@ def exec_model_in_container(self, model_path, cmd_args, args):
     def setup_mounts(self, model_path, args):
         if args.runtime == "vllm":
             model_base = ""
-            if self.model_store and hasattr(self, 'model_tag'):
-                ref_file = self.model_store.get_ref_file(self.model_tag)
+            if self.model_store and getattr(self, 'model_tag', None):
+                ref_file = self.store.get_ref_file(self.model_tag)
                 if ref_file and hasattr(ref_file, 'hash'):
                     model_base = self.model_store.model_base_directory
             if not model_base and (model_path and os.path.exists(model_path)):
@@ -457,18 +457,22 @@ def build_exec_args_bench(self, args, model_path):
         return exec_args
 
     def validate_args(self, args):
+        # If --container was specified return valid
         if args.container:
             return
         if args.privileged:
             raise KeyError(
                 "--nocontainer and --privileged options conflict. The --privileged option requires a container."
             )
-        if hasattr(args, "name") and args.name:
-            if hasattr(args, "generate"):
-                # Do not fail on serve if user specified --generate
-                if args.generate:
-                    return
-            raise KeyError("--nocontainer and --name options conflict. The --name option requires a container.")
+        # If --name was not specified return valid
+        if not getattr(args, "name", None):
+            return
+        # If --generate was specified return valid
+        if getattr(args, "generate", False):
+            # Do not fail on serve if user specified --generate
+            return
+
+        raise KeyError("--nocontainer and --name options conflict. The --name option requires a container.")
 
     def vllm_serve(self, args, exec_model_path):
         exec_args = [
@@ -525,7 +529,7 @@ def llama_serve(self, args, exec_model_path, chat_template_path, mmproj_path):
         if args.debug:
             exec_args += ["-v"]
 
-        if hasattr(args, "webui") and args.webui == "off":
+        if getattr(args, "webui", "") == "off":
             exec_args.extend(["--no-webui"])
 
         if check_nvidia() or check_metal(args):
@@ -576,7 +580,7 @@ def handle_runtime(self, args, exec_args, exec_model_path):
                 self.model_name,
             ]
 
-            if hasattr(args, 'runtime_args') and args.runtime_args:
+            if getattr(args, 'runtime_args', None):
                 exec_args.extend(args.runtime_args)
         else:
             gpu_args = self.gpu_args(args=args)
@@ -653,7 +657,7 @@ def serve(self, args, quiet=False):
             return
 
         # Add rag chatbot
-        if hasattr(args, "rag") and args.rag:
+        if getattr(args, "rag", None):
             exec_args = [
                 "bash",
                 "-c",
@@ -734,7 +738,7 @@ def get_available_port_if_any() -> int:
 
 def compute_serving_port(args, quiet=False) -> str:
     # user probably specified a custom port, don't override the choice
-    if hasattr(args, "port") and args.port not in ["", str(DEFAULT_PORT)]:
+    if getattr(args, "port", "") not in ["", str(DEFAULT_PORT)]:
         target_port = args.port
     else:
         # otherwise compute a random serving port in the range

ramalama/model_factory.py

@@ -36,7 +36,7 @@ def __init__(
 
         self.pruned_model = self.prune_model_input()
         self.draft_model = None
-        if hasattr(args, 'model_draft') and args.model_draft:
+        if getattr(args, 'model_draft', None):
             dm_args = copy.deepcopy(args)
             dm_args.model_draft = None
             self.draft_model = ModelFactory(args.model_draft, dm_args, ignore_stderr=True).create()
@@ -133,7 +133,7 @@ def create_oci(self) -> OCI:
     def create_url(self) -> URL:
         model = URL(self.pruned_model, self.store_path, urlparse(self.model).scheme)
         model.draft_model = self.draft_model
-        if hasattr(self, 'split_model'):
+        if getattr(self, 'split_model', None):
             model.split_model = self.split_model
             model.mnt_path = self.mnt_path
         return model

ramalama/oci.py

@@ -178,7 +178,7 @@ def _generate_containerfile(self, source_model, args):
         # Generate the containerfile content
         # Keep this in sync with docs/ramalama-oci.5.md !
         is_car = args.type == "car"
-        has_gguf = hasattr(args, 'gguf') and args.gguf is not None
+        has_gguf = getattr(args, 'gguf', None) is not None
         content = ""
 
         model_name = source_model.model_name

ramalama/quadlet.py

@@ -6,9 +6,7 @@
 
 class Quadlet:
     def __init__(self, model, chat_template, args, exec_args):
-        self.ai_image = model
-        if hasattr(args, "MODEL"):
-            self.ai_image = args.MODEL
+        self.ai_image = getattr(args, "MODEL", model)
         self.ai_image = self.ai_image.removeprefix("oci://")
         if args.name:
             self.name = args.name
@@ -83,7 +81,7 @@ def _gen_image(self, name, image):
         return image_file
 
     def _gen_name(self, quadlet_file: UnitFile):
-        if hasattr(self.args, "name") and self.args.name:
+        if getattr(self.args, "name", None):
             quadlet_file.add("Container", "ContainerName", f"{self.args.name}")
 
     def _gen_model_volume(self, quadlet_file: UnitFile):
@@ -111,13 +109,13 @@ def _gen_model_volume(self, quadlet_file: UnitFile):
         return files
 
     def _gen_port(self, quadlet_file: UnitFile):
-        if hasattr(self.args, "port") and self.args.port != "":
+        if getattr(self.args, "port", "") != "":
             quadlet_file.add("Container", "PublishPort", f"{self.args.port}:{self.args.port}")
 
     def _gen_rag_volume(self, quadlet_file: UnitFile):
         files: list[UnitFile] = []
 
-        if not hasattr(self.args, "rag") or not self.rag:
+        if not getattr(self.args, "rag", None):
             return files
 
         rag_volume_file_name = f"{self.rag_name}.volume"

ramalama/stack.py

@@ -20,7 +20,7 @@ class Stack:
 
     def __init__(self, args):
         self.args = args
-        self.name = args.name if hasattr(args, "name") and args.name else genname()
+        self.name = getattr(args, "name", None) or genname()
         if os.path.basename(args.engine) != "podman":
             raise ValueError("llama-stack requires use of the Podman container engine")
         self.host = "127.0.0.1"

test/system/050-pull.bats

@@ -77,6 +77,7 @@ load setup_suite
     is "$output" ".*Felladrin/gguf-smollm-360M-instruct-add-basics/smollm-360M-instruct-add-basics.IQ2_XXS" "image was actually pulled locally"
     run_ramalama rm huggingface://Felladrin/gguf-smollm-360M-instruct-add-basics/smollm-360M-instruct-add-basics.IQ2_XXS.gguf
 
+    skip_if_no_hf-cli
     run_ramalama pull hf://TinyLlama/TinyLlama-1.1B-Chat-v1.0
     run_ramalama list
     is "$output" ".*TinyLlama/TinyLlama-1.1B-Chat-v1.0" "image was actually pulled locally"