From ce595492b587a344d4485c53ba78c8a2e7ddc753 Mon Sep 17 00:00:00 2001 From: Hugo Herter Date: Mon, 25 Sep 2023 12:15:20 +0200 Subject: [PATCH] Cleanup: Code quality issues reported by tooling. This fixes a series of small code quality issues raised by our code quality tools. --- .github/workflows/code-quality.yml | 2 +- docker/run_vm_supervisor.sh | 4 +++- examples/example_http_js/src/run.sh | 3 +++ packaging/extract_requirements.sh | 2 +- runtimes/aleph-debian-11-python/init0.sh | 2 +- runtimes/aleph-debian-11-python/init1.py | 2 +- runtimes/instance-rootfs/create-debian-11-disk.sh | 2 +- runtimes/instance-rootfs/create-debian-12-disk.sh | 2 +- vm_supervisor/conf.py | 2 +- vm_supervisor/metrics.py | 4 ++-- vm_supervisor/storage.py | 1 - vm_supervisor/views/__init__.py | 2 +- vm_supervisor/views/operator.py | 6 ++---- vm_supervisor/vm/firecracker/executable.py | 15 +++++++-------- vm_supervisor/vm/firecracker/program.py | 2 +- 15 files changed, 26 insertions(+), 25 deletions(-) diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml index 97c209941..117ee7ef6 100644 --- a/.github/workflows/code-quality.yml +++ b/.github/workflows/code-quality.yml @@ -21,7 +21,7 @@ jobs: - name: Install required Python packages run: | - python3 -m pip install mypy pytest black isort + python3 -m pip install mypy pytest black isort flake8 - name: Test with Black run: | diff --git a/docker/run_vm_supervisor.sh b/docker/run_vm_supervisor.sh index 7b076783c..5499dc324 100755 --- a/docker/run_vm_supervisor.sh +++ b/docker/run_vm_supervisor.sh @@ -1,5 +1,7 @@ #!/bin/sh +set -euf + # Use Podman if installed, else use Docker if hash podman 2> /dev/null then @@ -17,4 +19,4 @@ $DOCKER_COMMAND run -ti --rm \ -v "$(pwd)/firecracker:/opt/aleph-vm/firecracker:ro" \ --device /dev/kvm \ -p 4020:4020 \ - alephim/vm-supervisor-dev $@ + alephim/vm-supervisor-dev "$@" diff --git a/examples/example_http_js/src/run.sh b/examples/example_http_js/src/run.sh index d56a2caf4..18430a26a 100755 --- a/examples/example_http_js/src/run.sh +++ b/examples/example_http_js/src/run.sh @@ -1,3 +1,6 @@ #!/bin/sh + +set -euf + cd /opt/code node /opt/code/server.js diff --git a/packaging/extract_requirements.sh b/packaging/extract_requirements.sh index aad72b7de..d443a0099 100755 --- a/packaging/extract_requirements.sh +++ b/packaging/extract_requirements.sh @@ -5,4 +5,4 @@ export DEBIAN_FRONTEND=noninteractive apt update apt --yes install /opt/packaging/target/aleph-vm.deb -pip freeze > $1 +pip freeze > "$1" diff --git a/runtimes/aleph-debian-11-python/init0.sh b/runtimes/aleph-debian-11-python/init0.sh index 8eb1b62bf..75659b0b9 100644 --- a/runtimes/aleph-debian-11-python/init0.sh +++ b/runtimes/aleph-debian-11-python/init0.sh @@ -5,7 +5,7 @@ set -euf mount -t proc proc /proc -o nosuid,noexec,nodev log() { - echo "$(cat /proc/uptime | awk '{printf $1}')" '|S' "$@" + echo "$(awk '{print $1}' /proc/uptime)" '|S' "$@" } log "init0.sh is launching" diff --git a/runtimes/aleph-debian-11-python/init1.py b/runtimes/aleph-debian-11-python/init1.py index 006109aaa..26747a7b1 100644 --- a/runtimes/aleph-debian-11-python/init1.py +++ b/runtimes/aleph-debian-11-python/init1.py @@ -32,7 +32,7 @@ logger.debug("Imports finished") __version__ = "2.0.0" -ASGIApplication = NewType("ASGIApplication", Any) +ASGIApplication = NewType("ASGIApplication", Any) # type: ignore class Encoding(str, Enum): diff --git a/runtimes/instance-rootfs/create-debian-11-disk.sh b/runtimes/instance-rootfs/create-debian-11-disk.sh index 5d7b51fd4..1ee49c8b0 100755 --- a/runtimes/instance-rootfs/create-debian-11-disk.sh +++ b/runtimes/instance-rootfs/create-debian-11-disk.sh @@ -35,7 +35,7 @@ tar xvf "$IMAGE_NAME" # Mount first partition of Debian Image LOOPDISK=$(losetup --find --show $IMAGE_RAW_NAME) -partx -u $LOOPDISK +partx -u "$LOOPDISK" mount "$LOOPDISK"p1 "$MOUNT_ORIGIN_DIR" # Fix boot partition missing diff --git a/runtimes/instance-rootfs/create-debian-12-disk.sh b/runtimes/instance-rootfs/create-debian-12-disk.sh index 3078c5c26..cfa0130a5 100755 --- a/runtimes/instance-rootfs/create-debian-12-disk.sh +++ b/runtimes/instance-rootfs/create-debian-12-disk.sh @@ -35,7 +35,7 @@ tar xvf "$IMAGE_NAME" # Mount first partition of Debian Image LOOPDISK=$(losetup --find --show $IMAGE_RAW_NAME) -partx -u $LOOPDISK +partx -u "$LOOPDISK" mount "$LOOPDISK"p1 "$MOUNT_ORIGIN_DIR" # Fix boot partition missing diff --git a/vm_supervisor/conf.py b/vm_supervisor/conf.py index e4a46f0da..cc55448d0 100644 --- a/vm_supervisor/conf.py +++ b/vm_supervisor/conf.py @@ -271,7 +271,7 @@ def check(self): assert isfile(self.FIRECRACKER_PATH), f"File not found {self.FIRECRACKER_PATH}" assert isfile(self.JAILER_PATH), f"File not found {self.JAILER_PATH}" assert isfile(self.LINUX_PATH), f"File not found {self.LINUX_PATH}" - assert self.NETWORK_INTERFACE, f"Network interface is not specified" + assert self.NETWORK_INTERFACE, "Network interface is not specified" assert self.CONNECTOR_URL.startswith( "http://" ) or self.CONNECTOR_URL.startswith("https://") diff --git a/vm_supervisor/metrics.py b/vm_supervisor/metrics.py index a5687bbb7..8f54aeba3 100644 --- a/vm_supervisor/metrics.py +++ b/vm_supervisor/metrics.py @@ -67,7 +67,7 @@ async def save_execution_data(execution_uuid: UUID, execution_data: str): async def save_record(record: ExecutionRecord): """Record the resource usage in database""" - session = Session() + session = Session() # noqa: F821 undefined name 'Session' try: session.add(record) session.commit() @@ -77,7 +77,7 @@ async def save_record(record: ExecutionRecord): async def get_execution_records() -> Iterable[ExecutionRecord]: """Get the execution records from the database.""" - session = Session() + session = Session() # noqa: F821 undefined name 'Session' try: return session.query(ExecutionRecord).all() finally: diff --git a/vm_supervisor/storage.py b/vm_supervisor/storage.py index 4d9db0d6b..51e71ee20 100644 --- a/vm_supervisor/storage.py +++ b/vm_supervisor/storage.py @@ -9,7 +9,6 @@ import re import sys from datetime import datetime -from enum import Enum from pathlib import Path from shutil import copy2, disk_usage, make_archive from typing import Union diff --git a/vm_supervisor/views/__init__.py b/vm_supervisor/views/__init__.py index bc034f21d..5165a5b92 100644 --- a/vm_supervisor/views/__init__.py +++ b/vm_supervisor/views/__init__.py @@ -219,7 +219,7 @@ async def update_allocations(request: web.Request): for execution in pool.get_persistent_executions(): if execution.vm_hash not in allocations: vm_type = "instance" if execution.is_instance else "persistent program" - logger.info(f"Stopping %s %s", vm_type, execution.vm_hash) + logger.info("Stopping %s %s", vm_type, execution.vm_hash) await execution.stop() execution.persistent = False diff --git a/vm_supervisor/views/operator.py b/vm_supervisor/views/operator.py index e7d0cf616..6bbd4ec9d 100644 --- a/vm_supervisor/views/operator.py +++ b/vm_supervisor/views/operator.py @@ -1,4 +1,5 @@ import asyncio +import functools import json import logging from datetime import datetime, timedelta @@ -19,9 +20,6 @@ logger = logging.getLogger(__name__) -import functools - - def is_token_still_valid(timestamp): """ Checks if a token has exprired based on its timestamp @@ -199,7 +197,7 @@ async def operate_stop(request: web.Request): execution.persistent = False return web.Response(status=200, body=f"Stopped VM with ref {vm_hash}") else: - return web.Response(status=200, body=f"Already stopped, nothing to do") + return web.Response(status=200, body="Already stopped, nothing to do") @require_jwk_authentication diff --git a/vm_supervisor/vm/firecracker/executable.py b/vm_supervisor/vm/firecracker/executable.py index b179624f2..0d3985d53 100644 --- a/vm_supervisor/vm/firecracker/executable.py +++ b/vm_supervisor/vm/firecracker/executable.py @@ -9,16 +9,10 @@ from multiprocessing import Process, set_start_method from os.path import exists, isfile from pathlib import Path -from typing import Any, Dict, Generic, List, Optional, TypeVar +from typing import Dict, Generic, List, Optional, TypeVar -from aleph_message.models import ItemHash - -psutil: Optional[Any] -try: - import psutil # type: ignore [no-redef] -except ImportError: - psutil = None from aiohttp import ClientResponseError +from aleph_message.models import ItemHash from aleph_message.models.execution.environment import MachineResources from firecracker.config import FirecrackerConfig @@ -31,6 +25,11 @@ from vm_supervisor.snapshots import CompressedDiskVolumeSnapshot from vm_supervisor.storage import get_volume_path +try: + import psutil # type: ignore [no-redef] +except ImportError: + psutil = None + logger = logging.getLogger(__name__) set_start_method("spawn") diff --git a/vm_supervisor/vm/firecracker/program.py b/vm_supervisor/vm/firecracker/program.py index 61e1b2ade..cb21b487f 100644 --- a/vm_supervisor/vm/firecracker/program.py +++ b/vm_supervisor/vm/firecracker/program.py @@ -63,7 +63,7 @@ def read_input_data(path_to_data: Optional[Path]) -> Optional[bytes]: return None if os.path.getsize(path_to_data) > settings.MAX_DATA_ARCHIVE_SIZE: - raise FileTooLargeError(f"Data file too large to pass as an inline zip") + raise FileTooLargeError("Data file too large to pass as an inline zip") return path_to_data.read_bytes()