Merge remote-tracking branch 'origin/main' into start-run-in-foregrou…

…nd-option

.github/workflows/pytest.yml

@@ -0,0 +1,29 @@
+name: Unit Tests
+
+on:
+  push:
+    branches: [ main ]
+  pull_request:
+    branches: [ main ]
+
+jobs:
+  unit_tests:
+
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        python-version: ["pypy3.9", "pypy3.10", "3.9", "3.10", "3.11", "3.12"]
+    steps:
+      - uses: actions/checkout@v4
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: ${{matrix.python-version}}
+      - name: Install dependencies
+        run: |
+          python -m pip install --upgrade pip
+          python -m pip install .
+          pip install pytest pytest-cov
+      - name: Test with pytest
+        run: |
+          pytest -v

pyproject.toml

@@ -5,7 +5,7 @@ build-backend = "setuptools.build_meta"
 [project]
 name = "qlever"
 description = "Script for using the QLever SPARQL engine."
-version = "0.5.16"
+version = "0.5.17"
 authors = [
     { name = "Hannah Bast", email = "[email protected]" }
 ]
@@ -35,3 +35,8 @@ package-data = { "qlever" = ["Qleverfiles/*"] }
 
 [tool.pytest.ini_options]
 pythonpath = ["src"]
+
+[tool.ruff]
+line-length = 79 
+[tool.ruff.lint]
+extend-select = ["I"]

src/qlever/commands/index.py

@@ -147,7 +147,7 @@ def get_input_options_for_json(self, args) -> str:
                 raise self.InvalidInputJson(
                     f"Element {i} in `MULTI_INPUT_JSON` must only contain "
                     "the keys `format`, `graph`, and `parallel`. Contains "
-                    "extra keys {extra_keys}.",
+                    f"extra keys {extra_keys}.",
                     input_spec,
                 )
             # Add the command-line options for this input stream. We use

src/qlever/commands/start.py

@@ -13,6 +13,110 @@
 from qlever.util import is_qlever_server_alive, run_command
 
 
+# Construct the command line based on the config file.
+def construct_command(args) -> str:
+    start_cmd = (
+        f"{args.server_binary}"
+        f" -i {args.name}"
+        f" -j {args.num_threads}"
+        f" -p {args.port}"
+        f" -m {args.memory_for_queries}"
+        f" -c {args.cache_max_size}"
+        f" -e {args.cache_max_size_single_entry}"
+        f" -k {args.cache_max_num_entries}"
+    )
+
+    if args.timeout:
+        start_cmd += f" -s {args.timeout}"
+    if args.access_token:
+        start_cmd += f" -a {args.access_token}"
+    if args.only_pso_and_pos_permutations:
+        start_cmd += " --only-pso-and-pos-permutations"
+    if not args.use_patterns:
+        start_cmd += " --no-patterns"
+    if args.use_text_index == "yes":
+        start_cmd += " -t"
+    start_cmd += f" > {args.name}.server-log.txt 2>&1"
+    return start_cmd
+
+
+# Kill existing server on the same port. Trust that StopCommand() works?
+# Maybe return StopCommand().execute(args) and handle it with a try except?
+def kill_existing_server(args) -> bool:
+    args.cmdline_regex = f"^ServerMain.* -p {args.port}"
+    args.no_containers = True
+    if not StopCommand().execute(args):
+        log.error("Stopping the existing server failed")
+        return False
+    log.info("")
+    return True
+
+
+# Run the command in a container
+def wrap_command_in_container(args, start_cmd) -> str:
+    if not args.server_container:
+        args.server_container = f"qlever.server.{args.name}"
+    start_cmd = Containerize().containerize_command(
+        start_cmd,
+        args.system,
+        "run -d --restart=unless-stopped",
+        args.image,
+        args.server_container,
+        volumes=[("$(pwd)", "/index")],
+        ports=[(args.port, args.port)],
+        working_directory="/index",
+    )
+    return start_cmd
+
+
+# When running natively, check if the binary exists and works.
+def check_binary(binary) -> bool:
+    try:
+        run_command(f"{binary} --help")
+        return True
+    except Exception as e:
+        log.error(
+            f'Running "{binary}" failed, '
+            f"set `--server-binary` to a different binary or "
+            f"set `--system to a container system`"
+        )
+        log.info("")
+        log.info(f"The error message was: {e}")
+        return False
+
+
+# Set the index description.
+def set_index_description(access_arg, port, desc) -> bool:
+    curl_cmd = (
+        f"curl -Gs http://localhost:{port}/api"
+        f' --data-urlencode "index-description={desc}"'
+        f" {access_arg} > /dev/null"
+    )
+    log.debug(curl_cmd)
+    try:
+        run_command(curl_cmd)
+    except Exception as e:
+        log.error(f"Setting the index description failed ({e})")
+        return False
+    return True
+
+
+# Set the text description.
+def set_text_description(access_arg, port, text_desc) -> bool:
+    curl_cmd = (
+        f"curl -Gs http://localhost:{port}/api"
+        f' --data-urlencode "text-description={text_desc}"'
+        f" {access_arg} > /dev/null"
+    )
+    log.debug(curl_cmd)
+    try:
+        run_command(curl_cmd)
+    except Exception as e:
+        log.error(f"Setting the text description failed ({e})")
+        return False
+    return True
+
+
 class StartCommand(QleverCommand):
     """
     Class for executing the `start` command.
@@ -94,51 +198,18 @@ def execute(self, args) -> bool:
 
         # Kill existing server on the same port if so desired.
         if args.kill_existing_with_same_port:
-            args.cmdline_regex = f"^ServerMain.* -p {args.port}"
-            args.no_containers = True
-            if not StopCommand().execute(args):
-                log.error("Stopping the existing server failed")
+            if args.kill_existing_with_same_port and not kill_existing_server(
+                args
+            ):
                 return False
-            log.info("")
 
         # Construct the command line based on the config file.
-        start_cmd = (
-            f"{args.server_binary}"
-            f" -i {args.name}"
-            f" -j {args.num_threads}"
-            f" -p {args.port}"
-            f" -m {args.memory_for_queries}"
-            f" -c {args.cache_max_size}"
-            f" -e {args.cache_max_size_single_entry}"
-            f" -k {args.cache_max_num_entries}"
-        )
-        if args.timeout:
-            start_cmd += f" -s {args.timeout}"
-        if args.access_token:
-            start_cmd += f" -a {args.access_token}"
-        if args.only_pso_and_pos_permutations:
-            start_cmd += " --only-pso-and-pos-permutations"
-        if not args.use_patterns:
-            start_cmd += " --no-patterns"
-        if args.use_text_index == "yes":
-            start_cmd += " -t"
-        start_cmd += f" > {args.name}.server-log.txt 2>&1"
+        start_cmd = construct_command(args)
 
         # Run the command in a container (if so desired). Otherwise run with
         # `nohup` so that it keeps running after the shell is closed.
         if args.system in Containerize.supported_systems():
-            if not args.server_container:
-                args.server_container = f"qlever.server.{args.name}"
-            start_cmd = Containerize().containerize_command(
-                start_cmd,
-                args.system,
-                "run -d --restart=unless-stopped",
-                args.image,
-                args.server_container,
-                volumes=[("$(pwd)", "/index")],
-                ports=[(args.port, args.port)],
-                working_directory="/index",
-            )
+            start_cmd = wrap_command_in_container(args, start_cmd)
         elif args.run_in_foreground:
             start_cmd = f"{start_cmd}"
         else:
@@ -151,16 +222,8 @@ def execute(self, args) -> bool:
 
         # When running natively, check if the binary exists and works.
         if args.system == "native":
-            try:
-                run_command(f"{args.server_binary} --help")
-            except Exception as e:
-                log.error(
-                    f'Running "{args.server_binary}" failed, '
-                    f"set `--server-binary` to a different binary or "
-                    f"set `--system to a container system`"
-                )
-                log.info("")
-                log.info(f"The error message was: {e}")
+            ret = check_binary(args.server_binary)
+            if not ret:
                 return False
 
         # Check if a QLever server is already running on this port.
@@ -178,7 +241,6 @@ def execute(self, args) -> bool:
             args.cmdline_regex = f"^ServerMain.* -p *{port}"
             log.info("")
             StatusCommand().execute(args)
-
             return False
 
         # Remove already existing container.
@@ -220,33 +282,15 @@ def execute(self, args) -> bool:
         while not is_qlever_server_alive(port):
             time.sleep(1)
 
-        # Set the access token if specified.
+        # Set the description for the index and text.
         access_arg = f'--data-urlencode "access-token={args.access_token}"'
         if args.description:
-            desc = args.description
-            curl_cmd = (
-                f"curl -Gs http://localhost:{port}/api"
-                f' --data-urlencode "index-description={desc}"'
-                f" {access_arg} > /dev/null"
-            )
-            log.debug(curl_cmd)
-            try:
-                run_command(curl_cmd)
-            except Exception as e:
-                log.error(f"Setting the index description failed ({e})")
+            ret = set_index_description(access_arg, port, args.description)
+            if not ret:
                 return False
         if args.text_description:
-            text_desc = args.text_description
-            curl_cmd = (
-                f"curl -Gs http://localhost:{port}/api"
-                f' --data-urlencode "text-description={text_desc}"'
-                f" {access_arg} > /dev/null"
-            )
-            log.debug(curl_cmd)
-            try:
-                run_command(curl_cmd)
-            except Exception as e:
-                log.error(f"Setting the text description failed ({e})")
+            ret = set_text_description(access_arg, port, args.text_description)
+            if not ret:
                 return False
 
         # Kill the tail process. NOTE: `tail_proc.kill()` does not work.

src/qlever/commands/stop.py

@@ -1,15 +1,40 @@
 from __future__ import annotations
-
 import re
-
 import psutil
-
 from qlever.command import QleverCommand
 from qlever.commands.status import StatusCommand
 from qlever.containerize import Containerize
 from qlever.log import log
 from qlever.util import show_process_info
 
+# try to kill the given process, return true iff it was killed successfully.
+# the process_info is used for logging.
+def stop_process(proc, pinfo):
+    try:
+        proc.kill()
+        log.info(f"Killed process {pinfo['pid']}")
+        return True
+    except Exception as e:
+        log.error(f"Could not kill process with PID "
+                  f"{pinfo['pid']} ({e}) ... try to kill it "
+                  f"manually")
+        log.info("")
+        show_process_info(proc, "", show_heading=True)
+        return False
+
+
+# try to stop and remove container. return True iff it was stopped
+# successfully. Gives log info accordingly.
+def stop_container(server_container):
+    for container_system in Containerize.supported_systems():
+        if Containerize.stop_and_remove_container(
+                container_system, server_container):
+            log.info(f"{container_system.capitalize()} container with "
+                     f"name \"{server_container}\" stopped "
+                     f" and removed")
+            return True
+    return False
+
 
 class StopCommand(QleverCommand):
     """
@@ -20,7 +45,7 @@ def __init__(self):
         pass
 
     def description(self) -> str:
-        return ("Stop QLever server for a given datasedataset or port")
+        return "Stop QLever server for a given datasedataset or port"
 
     def should_have_qleverfile(self) -> bool:
         return True
@@ -54,46 +79,31 @@ def execute(self, args) -> bool:
         # First check if there is container running and if yes, stop and remove
         # it (unless the user has specified `--no-containers`).
         if not args.no_containers:
-            for container_system in Containerize.supported_systems():
-                if Containerize.stop_and_remove_container(
-                        container_system, args.server_container):
-                    log.info(f"{container_system.capitalize()} container with "
-                             f"name \"{args.server_container}\" stopped "
-                             f" and removed")
-                    return True
+            if stop_container(args.server_container):
+                return True
 
         # Check if there is a process running on the server port using psutil.
-        #
         # NOTE: On MacOS, some of the proc's returned by psutil.process_iter()
         # no longer exist when we try to access them, so we just skip them.
         for proc in psutil.process_iter():
             try:
                 pinfo = proc.as_dict(
-                        attrs=['pid', 'username', 'create_time',
-                               'memory_info', 'cmdline'])
+                    attrs=['pid', 'username', 'create_time',
+                           'memory_info', 'cmdline'])
                 cmdline = " ".join(pinfo['cmdline'])
             except Exception as e:
                 log.debug(f"Error getting process info: {e}")
+                return False
             if re.search(cmdline_regex, cmdline):
                 log.info(f"Found process {pinfo['pid']} from user "
                          f"{pinfo['username']} with command line: {cmdline}")
                 log.info("")
-                try:
-                    proc.kill()
-                    log.info(f"Killed process {pinfo['pid']}")
-                except Exception as e:
-                    log.error(f"Could not kill process with PID "
-                              f"{pinfo['pid']} ({e}) ... try to kill it "
-                              f"manually")
-                    log.info("")
-                    show_process_info(proc, "", show_heading=True)
-                    return False
-                return True
+                return stop_process(proc, pinfo)
 
         # If no matching process found, show a message and the output of the
         # status command.
         message = "No matching process found" if args.no_containers else \
-                  "No matching process or container found"
+            "No matching process or container found"
         log.error(message)
         args.cmdline_regex = "^ServerMain.* -i [^ ]*"
         log.info("")