Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance code quality checks #411

Merged
merged 2 commits into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 27 additions & 6 deletions .github/workflows/code-quality.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: Test code quality
on: push

jobs:
code-quality:
code-quality-python:
runs-on: ubuntu-22.04

steps:
Expand All @@ -21,21 +21,42 @@ 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: |
black --check ./vm_supervisor
black --check ./runtimes/aleph-debian-11-python/init1.py

- name: Test with isort
run: |
isort --check-only --profile=black ./vm_supervisor
isort --check-only --profile=black ./runtimes/aleph-debian-11-python/init1.py

- name: Test with MyPy
run: |
mypy --ignore-missing-imports ./vm_supervisor
# mypy --config-file ./mypy.ini ./vm_supervisor
mypy --ignore-missing-imports ./runtimes/aleph-debian-11-python/init1.py

# - name: Test with flake8
# run: |
# flake8 ./vm_supervisor
- name: Test with flake8
run: |
flake8 --extend-ignore E501 ./vm_supervisor
flake8 --extend-ignore E501,E402 ./runtimes/aleph-debian-11-python/init1.py

code-quality-shell:
runs-on: ubuntu-22.04

steps:
- uses: actions/checkout@v4

- name: Workaround github issue https://github.com/actions/runner-images/issues/7192
run: sudo echo RESET grub-efi/install_devices | sudo debconf-communicate grub-pc

- name: Install required system packages only for Ubuntu Linux
run: |
sudo apt-get update
sudo apt-get install -y shellcheck

- name: Run Shellcheck on all shell scripts
run: |
find ./ -type f -name "*.sh" -exec shellcheck {} \;
4 changes: 3 additions & 1 deletion docker/run_vm_supervisor.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#!/bin/sh

set -euf

# Use Podman if installed, else use Docker
if hash podman 2> /dev/null
then
Expand All @@ -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 "$@"
3 changes: 3 additions & 0 deletions examples/example_http_js/src/run.sh
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#!/bin/sh

set -euf

cd /opt/code
node /opt/code/server.js
2 changes: 1 addition & 1 deletion packaging/extract_requirements.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
2 changes: 1 addition & 1 deletion runtimes/aleph-debian-11-python/init0.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
2 changes: 1 addition & 1 deletion runtimes/aleph-debian-11-python/init1.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion runtimes/instance-rootfs/create-debian-11-disk.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion runtimes/instance-rootfs/create-debian-12-disk.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion vm_supervisor/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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://")
Expand Down
4 changes: 2 additions & 2 deletions vm_supervisor/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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:
Expand Down
1 change: 0 additions & 1 deletion vm_supervisor/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion vm_supervisor/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 2 additions & 4 deletions vm_supervisor/views/operator.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import asyncio
import functools
import json
import logging
from datetime import datetime, timedelta
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
15 changes: 7 additions & 8 deletions vm_supervisor/vm/firecracker/executable.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")

Expand Down
2 changes: 1 addition & 1 deletion vm_supervisor/vm/firecracker/program.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down