chore: Upgrade versions (#301)

* chore: Upgrade versions

* fix: Easy to fix errors in CI

* feature: Migarte from pg 11 to pg 13

* fix: Sorcery stuff

* feature: pg11->13

* fix: Too board exceptions

* fix: Comprehension to generator

* fix: Broad error catches

* fix: lint

* fix: lint

* fix: Finally, fix pytest on autochecker

* fix: Add some tests (+ I hate Celery testing)

* fix: Add tests for bootstrap and linter

* fix: Refactors and style fixes

* fix: We don't really have yarn.lock yet

* fix: NOQA on import subprocces

* fix: Test lints

* fix: Add noqa to fake pass

* fix: Add noqa to fake pass

Author: yammesicka

dev_requirements.txt

@@ -1,4 +1,4 @@
-debugpy==1.0.0rc2
-ipdb==0.13.3
-pytest-cov==2.10.1
+debugpy==1.4.3
+ipdb==0.13.9
+pytest-cov==2.12.1
 pytest-env==0.6.2

devops/build.sh

@@ -4,5 +4,8 @@ SCRIPT_FILE_PATH=$(readlink -f "${0}")
 SCRIPT_FOLDER=$(dirname "${SCRIPT_FILE_PATH}")
 MAIN_FOLDER="${SCRIPT_FOLDER}/.."
 
+echo "Asking for sudo to chown the rabbitmq cookie:"
+sudo chown "$USER:$USER" "$SCRIPT_FOLDER/rabbitmq.cookie"
+
 echo "Running build on folder ${MAIN_FOLDER}"
 ( cd "${MAIN_FOLDER}" && docker build -t lms . )

devops/dev_bootstrap.sh

@@ -15,6 +15,9 @@ CONFIG_FILE_PATH="${LMSWEB_FOLDER}/config.py"
 CONFIG_EXAMPLE_FILE_PATH="${LMSWEB_FOLDER}/config.py.example"
 DB_BOOTSTRAP_FILE_PATH="${LMSAPP_FOLDER}/lmsdb/bootstrap.py"
 
+python_exec=python
+pip_exec=pip
+
 if ! (test -f "${CONFIG_FILE_PATH}"); then
   echo "Creating config from template"
   cp "${CONFIG_EXAMPLE_FILE_PATH}" "${CONFIG_FILE_PATH}"

devops/lms.yml

@@ -2,34 +2,31 @@ version: "3"
 
 services:
   db:
-    image: postgres:11-alpine
+    image: postgres:13-alpine
     ports:
       - 5432:5432
     volumes:
       - db-data-volume:/pg_data
     environment:
-      - PGDATA=/pg_data
+      - PGDATA=/pg_data/13/data
       - POSTGRES_DB=${DB_NAME:-db}
       - POSTGRES_USER=${DB_USERNAME:-postgres}
       - POSTGRES_PASSWORD=${DB_PASSWORD:-postgres}
     healthcheck:
-      test: ["CMD-SHELL", "pg_isready", "-U", "$DB_NAME"]
+      test: ["CMD-SHELL", "pg_isready -U $DB_USERNAME -d $DB_NAME"]
       interval: 30s
       timeout: 30s
       retries: 3
     networks:
       - lms
 
   rabbitmq:
-    image: rabbitmq:3.7-management-alpine
+    image: rabbitmq:3.9-management-alpine
     hostname: celery-mq
-    environment:
-      - RABBITMQ_ERLANG_COOKIE=AAVyo5djdSMGIZXiwEQs3JeVaBx5l14z
-      - RABBITMQ_DEFAULT_USER=rabbit-user
-      - RABBITMQ_DEFAULT_PASS=YgKlCvnYVzpTa3T9adG3NrMoUNe4Z5aZ
-      - RABBITMQ_DEFAULT_VHOST=/
     volumes:
       - rabbit-data-volume:/var/lib/rabbitmq
+      - ./rabbitmq.conf:/etc/rabbitmq/rabbitmq.conf
+      - ./rabbitmq.cookie:/var/lib/rabbitmq/.erlang.cookie
     networks:
       - lms
 

devops/rabbitmq.conf

@@ -0,0 +1,3 @@
+default_user=rabbit-user
+default_pass=YgKlCvnYVzpTa3T9adG3NrMoUNe4Z5aZ
+default_vhost=/

devops/rabbitmq.cookie

@@ -0,0 +1 @@
+erlang_cookie=AAVyo5djdSMGIZXiwEQs3JeVaBx5l14z

devops/upgrade_pg/upgrade.sh

@@ -0,0 +1,45 @@
+docker stop lms_db_1
+
+# Put values here if you've configured them before
+# DB_NAME=lms
+# DB_USERNAME=lmsweb
+# DB_PASSWORD=
+
+CURRENT_DATE=$(date +%d-%m-%Y_%H_%M_%S)
+MOUNT_PATH=/pg_data
+PG_OLD_DATA=/pg_data/11/data
+PG_NEW_DATA=/pg_data/13/data
+BACKUP_FILENAME=v11.$CURRENT_DATE.sql
+BACKUP_PATH=$MOUNT_PATH/backup/$BACKUP_FILENAME
+BACKUP_DIR=$(dirname "$BACKUP_PATH")
+VOLUME_NAME=lms_db-data-volume
+
+# Step 1: Create a backup
+docker run --rm -v $VOLUME_NAME:$MOUNT_PATH \
+       -e PGDATA=$PG_OLD_DATA \
+       -e POSTGRES_DB="${DB_NAME:-db}" \
+       -e POSTGRES_USER="${DB_USERNAME:-postgres}" \
+       -e POSTGRES_PASSWORD="${DB_PASSWORD:-postgres}" \
+       postgres:11-alpine \
+       /bin/bash -c "chown -R postgres:postgres $MOUNT_PATH \
+                && su - postgres /bin/bash -c \"/usr/local/bin/pg_ctl -D \\\"\$PGDATA\\\" start\" \
+                && mkdir -p \"$BACKUP_DIR\" \
+                && pg_dumpall -U $DB_USERNAME -f \"$BACKUP_PATH\" \
+                && chown postgres:postgres \"$BACKUP_PATH\""
+
+# Step 2: Create a new database from the backup
+docker run --rm -v $VOLUME_NAME:$MOUNT_PATH \
+       -e PGDATA=$PG_NEW_DATA \
+       -e POSTGRES_DB="${DB_NAME:-db}" \
+       -e POSTGRES_USER="${DB_USERNAME:-postgres}" \
+       -e POSTGRES_PASSWORD="${DB_PASSWORD:-postgres}" \
+       postgres:13-alpine \
+       /bin/bash -c "ls -la \"$BACKUP_DIR\" \
+                && mkdir -p \"\$PGDATA\" \
+                && chown -R postgres:postgres \"\$PGDATA\" \
+                && rm -rf $PG_NEW_DATA/* \
+                && su - postgres -c \"initdb -D \\\"\$PGDATA\\\"\" \
+                && su - postgres -c \"pg_ctl -D \\\"\$PGDATA\\\" -l logfile start\" \
+                && su - postgres -c \"psql -f $BACKUP_PATH\" \
+                && printf \"\\\nhost all all all md5\\\n\" >> \"\$PGDATA/pg_hba.conf\" \
+                "

lms/lmsdb/bootstrap.py

@@ -1,8 +1,8 @@
 from typing import Any, Callable, Optional, Tuple, Type
 
 from peewee import (
-    Entity, Expression, Field, Model, OP, OperationalError, ProgrammingError,
-    SQL,
+    Database, Entity, Expression, Field, Model, OP,
+    OperationalError, ProgrammingError, SQL,
 )
 from playhouse.migrate import migrate
 
@@ -215,12 +215,8 @@ def _drop_constraint_if_needed(table: Type[Model], column_name: str) -> bool:
     return True
 
 
-def has_column_named(table: Model, column_name: str) -> bool:
-    db = db_config.database
-    columns = {col.name for col in db.get_columns(table.__name__.lower())}
-    if column_name not in columns:
-        return False
-    return True
+def has_column_named(db: Database, table: Model, column_name: str) -> bool:
+    return column_name in table._meta.sorted_field_names
 
 
 def _add_api_keys_to_users_table(table: Model, _column: Field) -> None:

lms/lmstests/public/general/tasks.py

@@ -12,17 +12,14 @@
 
 @app.task
 def reset_solution_state_if_needed(solution_pk: str) -> None:
+    _logger.info('reset_solution_state_if_needed: solution %s', solution_pk)
+
     try:
-        _logger.info(
-            'Start reset_solution_state_if_needed solution %s',
-            solution_pk,
-        )
         solution = models.Solution.get_by_id(solution_pk)
-        if solution.state == models.Solution.STATES.IN_CHECKING.name:
-            _logger.info('Reset solution %s to CREATED state', solution_pk)
-            solution.set_state(models.Solution.STATES.CREATED)
-    except Exception:
-        _logger.exception(
-            'Failed reset_solution_state_if_needed solution %s',
-            solution_pk,
-        )
+    except models.Solution.DoesNotExist:
+        _logger.exception('Solution %s does not exist', solution_pk)
+        raise
+
+    if solution.state == models.Solution.STATES.IN_CHECKING.name:
+        _logger.info('Reset solution %s to CREATED state', solution_pk)
+        solution.set_state(models.Solution.STATES.CREATED)

lms/lmstests/public/identical_tests/services.py

@@ -65,13 +65,12 @@ def check_for_match_solutions_to_solve(self):
             return
 
         match_code = self.solution.solution_files.get().code
-        for solution_file in models.SolutionFile.select().join(
-                models.Solution,
-        ).filter(
+        files = models.SolutionFile.select().join(models.Solution).filter(
             models.Solution.exercise == self.solution.exercise,
             models.Solution.state == models.Solution.STATES.CREATED.name,
             models.SolutionFile.code == match_code,
-        ):
+        )
+        for solution_file in files:
             self._clone_solution_comments(
                 from_solution=self.solution,
                 to_solution=solution_file.solution,

lms/lmstests/public/identical_tests/tasks.py

@@ -2,6 +2,7 @@
 
 from celery.utils.log import get_task_logger
 
+from lms.lmsdb.models import Solution
 from lms.lmstests.public.config.celery import app
 from lms.lmstests.public.identical_tests import services
 
@@ -12,33 +13,33 @@
 
 @app.task
 def solve_solution_with_identical_code(solution_pk: str) -> None:
+    _logger.info(
+        'Start solve_solution_with_identical_code solution %s',
+        solution_pk,
+    )
+    checker = services.IdenticalSolutionSolver(solution_pk, _logger)
+
     try:
-        _logger.info(
-            'Start solve_solution_with_identical_code solution %s',
-            solution_pk,
-        )
-        checker = services.IdenticalSolutionSolver(solution_pk, _logger)
         checker.initialize()
-        checker.check_identical()
-    except Exception:
-        _logger.exception(
-            'Failed check_if_other_solutions_can_be_solved solution %s',
-            solution_pk,
-        )
+    except Solution.DoesNotExist:
+        _logger.exception('Solution %s does not exist', solution_pk)
+        raise
+
+    checker.check_identical()
 
 
 @app.task
 def check_if_other_solutions_can_be_solved(solution_pk: str) -> None:
+    _logger.info(
+        'Start check_if_other_solutions_can_be_solved solution %s',
+        solution_pk,
+    )
+    checker = services.IdenticalSolutionSolver(solution_pk, _logger)
+
     try:
-        _logger.info(
-            'Start check_if_other_solutions_can_be_solved solution %s',
-            solution_pk,
-        )
-        checker = services.IdenticalSolutionSolver(solution_pk, _logger)
         checker.initialize()
-        checker.check_for_match_solutions_to_solve()
-    except Exception:
-        _logger.exception(
-            'Failed check_if_other_solutions_can_be_solved solution %s',
-            solution_pk,
-        )
+    except Solution.DoesNotExist:
+        _logger.exception('Solution %s does not exist', solution_pk)
+        raise
+
+    checker.check_for_match_solutions_to_solve()

lms/lmstests/public/linters/services.py

@@ -16,11 +16,7 @@
 class LinterChecker:
     sandbox_tasks = linters.tasks
 
-    def __init__(
-            self,
-            solution_check_pk: str,
-            logger: logging.Logger,
-    ):
+    def __init__(self, solution_check_pk: str, logger: logging.Logger):
         self._solution_id = solution_check_pk
         self._app = None
         self._solution = None

lms/lmstests/public/linters/tasks.py

@@ -1,7 +1,9 @@
 import logging
 
+from celery.exceptions import OperationalError, TaskError
 from celery.utils.log import get_task_logger
 
+from lms.lmsdb import models
 from lms.lmstests.public.config.celery import app
 from lms.lmstests.public.linters import services
 
@@ -12,10 +14,16 @@
 
 @app.task
 def run_linter_on_solution(solution_pk: str) -> None:
+    _logger.info('Start running check solution %s', solution_pk)
+    checker = services.LinterChecker(solution_pk, _logger)
+
     try:
-        _logger.info('Start running check solution %s', solution_pk)
-        checker = services.LinterChecker(solution_pk, _logger)
         checker.initialize()
+    except models.Solution.DoesNotExist:
+        _logger.exception('Solution %s does not exist', solution_pk)
+        raise
+
+    try:
         checker.run_check()
-    except Exception:
+    except (TaskError, OperationalError):
         _logger.exception('Failed to check solution %s', solution_pk)

lms/lmstests/public/unittests/executers.py

@@ -50,7 +50,8 @@ def __enter__(self):
             '--rm', '--name', self._container_name, self.base_image,
             'sleep', str(self.timeout_seconds),
         )
-        log.info('Start executing safe container context with %s', args)
+        command = ' '.join(args)
+        log.info(f'Start executing safe container context with {command}')
         subprocess.check_call(args)  # NOQA: S603
         return self
 
@@ -89,9 +90,7 @@ def get_file(self, file_path: str):
 
 
 class SameProcessExecutor(BaseExecutor):
-    """
-    Used only for testing / local setups without docker
-    """
+    """Used only for testing / local setups without docker"""
     def __init__(self):
         self._cwd = tempfile.mkdtemp()
 
@@ -105,20 +104,22 @@ def run_on_executor(self, args: tuple):
         subprocess.run(args, cwd=self._cwd)  # NOQA: S603
 
     def write_file(self, file_path: str, content: str):
-        open(os.path.join(self._cwd, file_path), 'w').write(content)
+        with open(os.path.join(self._cwd, file_path), 'w') as f:
+            f.write(content)
 
     def get_file(self, file_path: str):
-        return open(os.path.join(self._cwd, file_path), 'r').read()
-
-
-__MAPPING = {
-    executor.executor_name(): executor
-    for executor in (
-        DockerExecutor,
-        SameProcessExecutor,
-    )
-}
+        file_content = ''
+        with open(os.path.join(self._cwd, file_path), 'r') as f:
+            file_content = f.read()
+        return file_content
 
 
 def get_executor(executor_name=None) -> BaseExecutor:
     return __MAPPING.get(executor_name, DockerExecutor)()
+
+
+EXECUTORS = (
+    DockerExecutor,
+    SameProcessExecutor,
+)
+__MAPPING = {executor.executor_name(): executor for executor in EXECUTORS}

lms/lmstests/public/unittests/import_tests.py

@@ -15,7 +15,7 @@ def register_test_class(file_path: str, test_class: typing.ClassVar):
     subject = test_class.__doc__
     exercise = models.Exercise.get_or_none(models.Exercise.subject == subject)
     if not exercise:
-        log.info('Failed to find exercise subject %s', subject)
+        log.info(f'Failed to find exercise subject {subject}')
         raise SystemError
 
     with open(file_path, 'r') as file_reader: