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

[WIP] Add first attempt at getting mssql support working #181

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
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
22 changes: 18 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ jobs:

strategy:
matrix:
python-version: ["3.7", "3.8"]
django-version: ["2.1.1", "3.1.4"]
database-engine: ["postgres", "mysql"]
python-version: [ "3.7", "3.8" ]
django-version: [ "2.1.1", "3.1.4" ]
database-engine: [ "postgres", "mysql", "mssql"]

services:
postgres:
Expand All @@ -37,6 +37,15 @@ jobs:
ports:
- 3306:3306


mssqldb:
image: "mcr.microsoft.com/mssql/server:2017-latest"
env:
ACCEPT_EULA: y
SA_PASSWORD: '~Test123'
ports:
- 1433:1433

steps:
- name: Checkout code
uses: actions/checkout@v2
Expand Down Expand Up @@ -73,11 +82,16 @@ jobs:
mysql_tzinfo_to_sql /usr/share/zoneinfo | mysql --protocol=TCP -h localhost -u root -prootpassword mysql
if: matrix.database-engine == 'mysql'

- name: Prepare mssql database
run: |
/opt/mssql-tools/bin/sqlcmd -S localhost -U sa -P '~Test123' -m-1 -Q 'CREATE DATABASE [binder-test];'
if: matrix.database-engine == 'mssql'

- name: Run tests
run: |
.venv/bin/coverage run --include="binder/*" setup.py test
env:
BINDER_TEST_MYSQL: ${{ matrix.database-engine == 'mysql' && 1 || 0 }}
BINDER_TEST_DATABASE_ENGINE: ${{ matrix.database-engine }}
CY_RUNNING_INSIDE_CI: 1

- name: Upload coverage report
Expand Down
10 changes: 9 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,19 @@ There are two ways to run the tests:
- Run with docker `docker-compose run binder ./setup.py test`
- Access the test database directly by with `docker-compose run db psql -h db -U postgres`.
- It may be possible to recreate the test database (for example when you added/changed models). One way of achieving this is to just remove all the docker images that were build `docker-compose rm`. The database will be created during the setup in `tests/__init__.py`.

The tests are set up in such a way that there is no need to keep migration files. The setup procedure in `tests/__init__.py` handles the preparation of the database by directly calling some build-in Django commands.

To only run a selection of the tests, use the `-s` flag like `./setup.py test -s tests.test_some_specific_test`.

### Running test other database systems
Locally the tests can be run for MySQL or MSSQL by overriding the `BINDER_TEST_DATABASE_ENGINE` environment variable:

- `docker-compose run -e BINDER_TEST_DATABASE_ENGINE=mysql binder ./setup.py test` for MySQL
- `docker-compose run -e BINDER_TEST_DATABASE_ENGINE=mssql binder ./setup.py test` for MsSQL



## MySQL support

MySQL is supported, but only with the goal to replace it with
Expand Down
1 change: 1 addition & 0 deletions binder/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ class Meta:

def save(self, *args, **kwargs):
self.full_clean() # Never allow saving invalid models!

return super().save(*args, **kwargs)


Expand Down
19 changes: 17 additions & 2 deletions binder/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -315,11 +315,18 @@ class ModelView(View):
# }
virtual_relations = {}

@property
def database_type(self):
return {
'mysql': 'mysql',
'microsoft': 'mssql',
}.get(connections[self.model.objects.db].vendor, 'postgres')

@property
def AggStrategy(self):
if connections[self.model.objects.db].vendor == 'mysql':
if self.database_type == 'mysql':
return GroupConcat
if connections[self.model.objects.db].vendor == 'microsoft':
if self.database_type == 'mssql':
return StringAgg
return OrderableArrayAgg

Expand Down Expand Up @@ -869,6 +876,14 @@ def _follow_related(self, fieldspec):
# permission scoping. This will be done when fetching the actual
# objects.
def _get_with_ids(self, pks, request, include_annotations, with_map, where_map):
if self.database_type == 'mssql':
with_ids = {}
for key, sub_with_map in with_map.items():
with_ids.update(self._base_get_with_ids(pks, request, include_annotations, {key: sub_with_map}, where_map))
return with_ids
return self._base_get_with_ids( pks, request, include_annotations, with_map, where_map)

def _base_get_with_ids(self, pks, request, include_annotations, with_map, where_map):
result = {}

annotations = {}
Expand Down
21 changes: 20 additions & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,28 @@ services:
db:
image: postgres:11.5
binder:
build: .
build:
context: ./
dockerfile: docker/binder/Dockerfile
command: tail -f /dev/null
volumes:
- .:/binder
depends_on:
- db
- mysql
- mssqldb
mysql:
image: mysql
environment:
MYSQL_ROOT_PASSWORD: rootpassword
ports:
- 3306:3306
mssqldb:
build:
context: ./
dockerfile: docker/mssqldb/Dockerfile
environment:
ACCEPT_EULA: y
SA_PASSWORD: '~Test123'
ports:
- 1433:1433
3 changes: 2 additions & 1 deletion Dockerfile → docker/binder/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
FROM python:3
FROM python:3-buster
ENV PYTHONUNBUFFERED 1
RUN mkdir /binder
WORKDIR /binder
ADD . /binder
RUN ./install-mssql-driver.sh
12 changes: 12 additions & 0 deletions docker/mssqldb/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
FROM mcr.microsoft.com/mssql/server:2017-latest
WORKDIR /

ENV ACCEPT_EULA Y
ENV SA_PASSWORD "~Test123"

COPY docker/mssqldb/init /init
COPY docker/mssqldb/bootstrap.sql /bootstrap.sql
COPY docker/mssqldb/entrypoint.sh /entrypoint.sh

EXPOSE 1433
CMD /entrypoint.sh
4 changes: 4 additions & 0 deletions docker/mssqldb/bootstrap.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
USE [master]
GO
CREATE DATABASE [binder-test]
GO
5 changes: 5 additions & 0 deletions docker/mssqldb/entrypoint.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/bin/sh

#start SQL Server, start the script to create the DB and import the data, start the app
nohup /init &
/opt/mssql/bin/sqlservr
7 changes: 7 additions & 0 deletions docker/mssqldb/init
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/bin/sh

#wait for the SQL Server to come up
sleep 20s

echo "Running CREATE DATABASE"
/opt/mssql-tools/bin/sqlcmd -S localhost -U sa -P '~Test123' -m-1 -i bootstrap.sql
14 changes: 14 additions & 0 deletions install-mssql-driver.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
curl https://packages.microsoft.com/keys/microsoft.asc | apt-key add -

#Download appropriate package for the OS version
#Choose only ONE of the following, corresponding to your OS version

#Debian 10
curl https://packages.microsoft.com/config/debian/10/prod.list > /etc/apt/sources.list.d/mssql-release.list

apt-get update
ACCEPT_EULA=Y apt-get install -y msodbcsql17
# optional: for bcp and sqlcmd
ACCEPT_EULA=Y apt-get install -y mssql-tools
echo 'export PATH="$PATH:/opt/mssql-tools/bin"' >> ~/.bashrc
apt-get install -y unixodbc-dev
4 changes: 3 additions & 1 deletion project/packages.pip
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,6 @@ Pillow
django-request-id
psycopg2
requests
openpyxl
openpyxl
django-pyodbc-azure
django-pyodbc
14 changes: 9 additions & 5 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@
# allow setup.py to be run from any path
os.chdir(os.path.normpath(os.path.join(os.path.abspath(__file__), os.pardir)))

test_require_database_engine = {
'mysql': ['mysqlclient >= 1.3.12', 'psycopg2 >= 2.7'],
'mssql': ['django-pyodbc-azure >= 2.1.0.0', 'django-pyodbc >= 1.1.3', 'django-hijack == 2.1.10', 'psycopg2 >= 2.7'],
# Alternative mssql setup with django 3 setup. For later
# 'mssql': ['mssql-django==1.0rc1', 'psycopg2 >= 2.7']
}.get(os.environ.get('BINDER_TEST_DATABASE_ENGINE'), ['psycopg2 >= 2.7'])


setup(
name='django-binder',
version='1.5.0',
Expand Down Expand Up @@ -44,11 +52,7 @@
],
tests_require=[
'django-hijack >= 2.1.10',
(
'mysqlclient >= 1.3.12'
if os.environ.get('BINDER_TEST_MYSQL', '0') == '1' else
'psycopg2 >= 2.7'
),
*test_require_database_engine,
"openpyxl >= 3.0.0"
],
)
54 changes: 40 additions & 14 deletions tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,12 @@
from django.core.management import call_command
import os

if (
os.path.exists('/.dockerenv') and
'CY_RUNNING_INSIDE_CI' not in os.environ
):
db_settings = {
'ENGINE': 'django.db.backends.postgresql',
'NAME': 'postgres',
'USER': 'postgres',
'HOST': 'db',
'PORT': 5432,
}
elif os.environ.get('BINDER_TEST_MYSQL', '0') == '1':
if os.environ.get('BINDER_TEST_DATABASE_ENGINE') == 'mssql':
os.system("/opt/mssql-tools/bin/sqlcmd -S localhost -U sa -P '~Test123' -m-1 -Q 'CREATE DATABASE [binder-test];'")



if os.environ.get('BINDER_TEST_DATABASE_ENGINE') == 'mysql':
db_settings = {
'ENGINE': 'django.db.backends.mysql',
'NAME': 'binder-test',
Expand All @@ -23,6 +17,26 @@
'USER': 'root',
'PASSWORD': 'rootpassword',
}
if os.environ.get('BINDER_TEST_DATABASE_ENGINE') == 'mssql':
db_settings = {
'ENGINE': 'sql_server.pyodbc',
'NAME': 'binder-test',
'TIME_ZONE': 'UTC',
'HOST': 'mssqldb',
'USER': 'sa',
'PASSWORD': '~Test123',
'OPTIONS': {
'driver': 'ODBC Driver 17 for SQL Server',
},
}
elif (os.path.exists('/.dockerenv') and 'CY_RUNNING_INSIDE_CI' not in os.environ):
db_settings = {
'ENGINE': 'django.db.backends.postgresql',
'NAME': 'postgres',
'USER': 'postgres',
'HOST': 'db',
'PORT': 5432,
}
else:
db_settings = {
'ENGINE': 'django.db.backends.postgresql_psycopg2',
Expand All @@ -31,6 +45,7 @@
'USER': 'postgres',
}


settings.configure(**{
'DEBUG': True,
'SECRET_KEY': 'testy mctestface',
Expand Down Expand Up @@ -74,6 +89,12 @@
'level': 'DEBUG',
'class': 'logging.StreamHandler',
},
'file': {
'level': 'DEBUG',
'class': 'logging.handlers.WatchedFileHandler',
'filename': 'backend.log',
'filters': [],
},
},
'loggers': {
# We override only this one to avoid logspam
Expand All @@ -83,7 +104,13 @@
'handlers': ['console'],
'level': 'ERROR',
},
}
# 'django.db.backends': {
# 'handlers': ['console', 'file'],
# 'level': 'DEBUG',
# 'propagate': True,
# },
}

},
'BINDER_PERMISSION': {
'default': [
Expand All @@ -110,7 +137,6 @@
'admin': []
}
})

setup()

# Do the dance to ensure the models are synched to the DB.
Expand Down
2 changes: 1 addition & 1 deletion tests/filters/test_time_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import os

# This is not possible in
if os.environ.get('BINDER_TEST_MYSQL', '0') != '1':
if os.environ.get('BINDER_TEST_DATABASE_ENGINE') not in ['mssql', 'mysql']:
class TimeFiltersTest(TestCase):

def setUp(self):
Expand Down
2 changes: 2 additions & 0 deletions tests/test_multi_put.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,8 @@ def test_put_relations_from_referencing_side(self):
}
response = self.client.put('/animal/', data=json.dumps(with_model_data), content_type='application/json')

print(response.content)

self.assertEqual(response.status_code, 200)

returned_data = jsonloads(response.content)
Expand Down
4 changes: 2 additions & 2 deletions tests/test_nulls_last.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def test_order_by_nulls_last(self):
self._load_test_data()

# MySQL has different defaults when no nulls option is selected...
if os.environ.get('BINDER_TEST_MYSQL', '0') != '0':
if os.environ.get('BINDER_TEST_DATABASE_ENGINE') in ['mssql', 'mysql']:
self._assert_order('last_seen', ['4', '5', '1', '2', '3'])
self._assert_order('-last_seen', ['3', '2', '1', '4', '5'])
else:
Expand All @@ -46,7 +46,7 @@ def test_order_by_nulls_last_on_annotation(self):
self._load_test_data()

# MySQL has different defaults when no nulls option is selected...
if os.environ.get('BINDER_TEST_MYSQL', '0') != '0':
if os.environ.get('BINDER_TEST_DATABASE_ENGINE') in ['mssql', 'mysql']:
self._assert_order('last_present', ['4', '5', '1', '2', '3'])
self._assert_order('-last_present', ['3', '2', '1', '4', '5'])
else:
Expand Down
4 changes: 2 additions & 2 deletions tests/test_postgres_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@
from binder.json import jsonloads
from django.contrib.auth.models import User

if os.environ.get('BINDER_TEST_MYSQL', '0') == '0':
if os.environ.get('BINDER_TEST_DATABASE_ENGINE') not in ['mysql', 'mssql']:
from .testapp.models import FeedingSchedule, Animal, Zoo

# TODO: Currently these only really test filtering. Move to test/filters?
@unittest.skipIf(
os.environ.get('BINDER_TEST_MYSQL', '0') != '0',
os.environ.get('BINDER_TEST_DATABASE_ENGINE') in ['mysql', 'mssql'],
"Only available with PostgreSQL"
)
class PostgresFieldsTest(TestCase):
Expand Down
2 changes: 1 addition & 1 deletion tests/testapp/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from .contact_person import ContactPerson
from .costume import Costume
# This is a Postgres-specific model
if os.environ.get('BINDER_TEST_MYSQL', '0') != '1':
if os.environ.get('BINDER_TEST_DATABASE_ENGINE') not in ['mysql', 'mssql']:
from .feeding_schedule import FeedingSchedule
from .gate import Gate
from .nickname import Nickname, NullableNickname
Expand Down
Loading