diff --git a/Makefile b/Makefile index 0ea319c35c9..96c52b724ae 100644 --- a/Makefile +++ b/Makefile @@ -1,3 +1,5 @@ +SHELL := /bin/bash + # List most target names as 'PHONY' to prevent Make from thinking it will be creating a file of the same name .PHONY: help clean clean-assets clean-build clean-pyc clean-docs lint test test-all assets coverage docs release test-namespaced-packages staticdeps staticdeps-cext writeversion setrequirements buildconfig pex i18n-extract-frontend i18n-extract-backend i18n-transfer-context i18n-extract i18n-django-compilemessages i18n-upload i18n-pretranslate i18n-pretranslate-approve-all i18n-download i18n-regenerate-fonts i18n-stats i18n-install-font i18n-download-translations i18n-download-glossary i18n-upload-glossary docker-whl docker-demoserver docker-devserver docker-envlist @@ -29,9 +31,11 @@ help: @echo "lint: check Python style with flake8" @echo "test: run tests quickly with the default Python" @echo "test-all: run tests on every Python version with Tox" + @echo "test-with-postgres: run tests quickly with a temporary postgresql backend" @echo "test-namespaced-packages: verify that we haven't fetched anything namespaced into kolibri/dist" @echo "coverage: run tests, recording and printing out Python code coverage" @echo "docs: generate developer documentation" + @echo "start-foreground-with-postgres: run Kolibri in foreground mode with a temporary postgresql backend" @echo "" @echo "Internationalization" @echo "--------------------" @@ -90,6 +94,29 @@ test: test-all: tox +%-with-postgres: + @echo -e "\e[33mWARNING: for testing purposes only; postgresql database backend is ephemeral\e[0m" + @echo -e "\e[36mINFO: run 'docker-compose -v' to remove the database volume\e[0m" + export KOLIBRI_DATABASE_ENGINE=postgres; \ + export KOLIBRI_DATABASE_NAME=default; \ + export KOLIBRI_DATABASE_USER=postgres; \ + export KOLIBRI_DATABASE_PASSWORD=postgres; \ + export KOLIBRI_DATABASE_HOST=127.0.0.1; \ + export KOLIBRI_DATABASE_PORT=5432; \ + set -ex; \ + function _on_interrupt() { docker-compose down; }; \ + trap _on_interrupt SIGINT SIGTERM SIGKILL ERR; \ + docker-compose up --detach; \ + until docker-compose logs --tail=1 postgres | grep -q "database system is ready to accept connections"; do \ + echo "$(date) - waiting for postgres..."; \ + sleep 1; \ + done; \ + $(MAKE) -e $(subst -with-postgres,,$@); \ + docker-compose down -v + +start-foreground: + kolibri start --foreground + assets: yarn install yarn run build diff --git a/docker-compose.yml b/docker-compose.yml new file mode 100644 index 00000000000..cce197fca9f --- /dev/null +++ b/docker-compose.yml @@ -0,0 +1,16 @@ +version: '3.4' + +services: + postgres: + image: postgres:12 + ports: + - "5432:5432" + environment: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: postgres + POSTGRES_DB: default + volumes: + - kolibri-pg:/var/lib/postgresql/data +# command: "postgres -c log_statement=all -c log_line_prefix='%t %d '" +volumes: + kolibri-pg: diff --git a/kolibri/core/auth/management/commands/sync.py b/kolibri/core/auth/management/commands/sync.py index edc102771d0..01132f18950 100644 --- a/kolibri/core/auth/management/commands/sync.py +++ b/kolibri/core/auth/management/commands/sync.py @@ -23,7 +23,7 @@ from kolibri.core.logger.utils.data import bytes_for_humans from kolibri.core.tasks.exceptions import UserCancelledError from kolibri.core.tasks.management.commands.base import AsyncCommand -from kolibri.core.utils.lock import db_lock +from kolibri.core.utils.lock import db_lock_sqlite_only from kolibri.utils import conf DATA_PORTAL_SYNCING_BASE_URL = conf.OPTIONS["Urls"]["DATA_PORTAL_SYNCING_BASE_URL"] @@ -216,7 +216,12 @@ def _lock(self): cancellable = self.job.cancellable self.job.save_as_cancellable(cancellable=False) - with db_lock(): + # Morango v0.6.13+ uses read repeatable isolation for postgres, and handles database + # transactions in a special manner that would be problematic if we began a transaction here. + # If data has changed during morango's transactions, postgres may throw a serialization + # rollback error, which will cause morango to retry it. So it's safe to avoid locking here + # when using postgres + with db_lock_sqlite_only(): yield if self.job: diff --git a/kolibri/core/auth/test/test_morango_integration.py b/kolibri/core/auth/test/test_morango_integration.py index 18a81d9513a..ea09c998497 100644 --- a/kolibri/core/auth/test/test_morango_integration.py +++ b/kolibri/core/auth/test/test_morango_integration.py @@ -7,6 +7,7 @@ import uuid from django.test import TestCase +from django.test import TransactionTestCase from morango.models import InstanceIDModel from morango.models import Store from morango.models import syncable_models @@ -44,7 +45,7 @@ def test_partition_and_id_values(self): self.assertTrue(partition.startswith(dataset_id)) -class DateTimeTZFieldTestCase(TestCase): +class DateTimeTZFieldTestCase(TransactionTestCase): def setUp(self): self.controller = MorangoProfileController(PROFILE_FACILITY_DATA) InstanceIDModel.get_or_create_current_instance() diff --git a/kolibri/core/utils/lock.py b/kolibri/core/utils/lock.py index 32502e63a4a..bab572187d5 100644 --- a/kolibri/core/utils/lock.py +++ b/kolibri/core/utils/lock.py @@ -30,8 +30,7 @@ def execute(self): @contextmanager -def db_lock(): - lock_id = 1 +def db_lock_sqlite_only(): if connection.vendor == "sqlite": while True: try: @@ -44,11 +43,30 @@ def db_lock(): except OperationalError as e: if "database is locked" not in str(e): raise e - elif connection.vendor == "postgresql": + else: + yield + + +@contextmanager +def db_lock_postgresql_only(): + lock_id = 1 + if connection.vendor == "postgresql": with transaction.atomic(): operation = PostgresLock(key=lock_id) operation.execute() yield + else: + yield + + +@contextmanager +def db_lock(): + if connection.vendor == "sqlite": + with db_lock_sqlite_only(): + yield + elif connection.vendor == "postgresql": + with db_lock_postgresql_only(): + yield else: raise NotImplementedError( "kolibri.core.utils.cache.DatabaseLock not implemented for vendor {vendor}".format( diff --git a/requirements/base.txt b/requirements/base.txt index fdf8fd29095..418e8aae3be 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -20,7 +20,7 @@ kolibri_exercise_perseus_plugin==1.3.5 jsonfield==2.0.2 requests-toolbelt==0.8.0 clint==0.5.1 -morango==0.6.13 +morango==0.6.14 tzlocal==2.1 pytz==2020.5 python-dateutil==2.7.5