From e4cb908c0278a7c70b717f56bcee13a236cfb2b7 Mon Sep 17 00:00:00 2001 From: kingbuzzman Date: Fri, 22 Jun 2018 13:36:47 +0200 Subject: [PATCH 1/6] Preliminary changes --- .../management/commands/migrate_schemas.py | 44 +++++++++++++++++-- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/tenant_schemas/management/commands/migrate_schemas.py b/tenant_schemas/management/commands/migrate_schemas.py index 7e131c2e..1fc4d0a7 100644 --- a/tenant_schemas/management/commands/migrate_schemas.py +++ b/tenant_schemas/management/commands/migrate_schemas.py @@ -1,6 +1,8 @@ +import math import django +import argparse + from django.core.management.commands.migrate import Command as MigrateCommand -from django.db import connection from tenant_schemas.management.commands import SyncCommon from tenant_schemas.migration_executors import get_executor @@ -13,6 +15,22 @@ class MigrationSchemaMissing(django.db.utils.DatabaseError): pass +def chunks(l, n): + for i in range(0, len(l), int(math.ceil(len(l) / n))): + yield l[i:i + n] + + +def greater_than_x(min_number, message): + def wrapper(astring): + if not astring.isdigit(): + raise argparse.ArgumentTypeError('Needs to be a number') + number = int(astring) + if not number > min_number: + raise argparse.ArgumentTypeError(message) + return number + return wrapper + + class Command(SyncCommon): help = "Updates database schema. Manages both apps with migrations and those without." @@ -28,9 +46,26 @@ def add_arguments(self, parser): super(Command, self).add_arguments(parser) command = MigrateCommand() command.add_arguments(parser) + parser.add_argument('--part', action='store', dest='migration_part', + type=greater_than_x(1, 'The number needs to be greated than one'), default=None, + help=('Splits the tenant schemas into pieces of equal parts to then be proccessed in ' + 'parts (requires --of).')) + parser.add_argument('--of', action='store', dest='migration_part_of', + type=greater_than_x(0, 'The number needs to be greated than zero'), default=None, + help='The part you want to process from the pieces (requires --part).') def handle(self, *args, **options): super(Command, self).handle(*args, **options) + + required_together = (self.options['migration_part'], self.options['migration_part_of'],) + if any(required_together) and not all(required_together): + raise Exception("--part and --of need to be used together.") + elif all(required_together): + if self.options['migration_part_of'] > self.options['migration_part']: + raise Exception("--of cannot be greater than --part.") + elif self.sync_public: + raise Exception("Cannot run public schema migrations along with --of and --part.") + self.PUBLIC_SCHEMA_NAME = get_public_schema_name() executor = get_executor(codename=self.executor)(self.args, self.options) @@ -48,6 +83,9 @@ def handle(self, *args, **options): else: tenants = [self.schema_name] else: - tenants = get_tenant_model().objects.exclude(schema_name=get_public_schema_name()).values_list( - 'schema_name', flat=True) + tenants = get_tenant_model().objects.exclude(schema_name=get_public_schema_name()) \ + .order_by('pk').values_list('schema_name', flat=True) + if self.options['migration_part'] and self.options['migration_part'] > 1 and tenants: + tenant_parts = list(chunks(tenants, self.options['migration_part'] - 1)) + tenants = tenant_parts[self.options['migration_part_of'] - 1] executor.run_migrations(tenants=tenants) From 6cc32250d05b261d726d6fd5fdee30b3269a91f8 Mon Sep 17 00:00:00 2001 From: buzzi Date: Fri, 22 Jun 2018 15:17:55 +0200 Subject: [PATCH 2/6] Fixes some more logic --- .../management/commands/migrate_schemas.py | 61 +++++++++++-------- 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/tenant_schemas/management/commands/migrate_schemas.py b/tenant_schemas/management/commands/migrate_schemas.py index 1fc4d0a7..60ef69fa 100644 --- a/tenant_schemas/management/commands/migrate_schemas.py +++ b/tenant_schemas/management/commands/migrate_schemas.py @@ -15,20 +15,25 @@ class MigrationSchemaMissing(django.db.utils.DatabaseError): pass -def chunks(l, n): - for i in range(0, len(l), int(math.ceil(len(l) / n))): - yield l[i:i + n] - - -def greater_than_x(min_number, message): - def wrapper(astring): - if not astring.isdigit(): - raise argparse.ArgumentTypeError('Needs to be a number') - number = int(astring) - if not number > min_number: - raise argparse.ArgumentTypeError(message) - return number - return wrapper +def chunks(tenants, total_parts): + """ + Iterates over tenants, returning each part, one at a time + """ + # import ipdb; print('\a'); ipdb.sset_trace() + if total_parts == 1: + yield tenants + tenants_per_chunk = int(math.ceil(float(len(tenants)) / total_parts)) + for i in range(0, len(tenants), tenants_per_chunk): + yield tenants[i:i + tenants_per_chunk] + + +def greater_than_zero(astring): + if not astring.isdigit(): + raise argparse.ArgumentTypeError('Needs to be a number') + number = int(astring) + if not number > 0: + raise argparse.ArgumentTypeError('The number needs to be greated than zero') + return number class Command(SyncCommon): @@ -46,22 +51,21 @@ def add_arguments(self, parser): super(Command, self).add_arguments(parser) command = MigrateCommand() command.add_arguments(parser) - parser.add_argument('--part', action='store', dest='migration_part', - type=greater_than_x(1, 'The number needs to be greated than one'), default=None, - help=('Splits the tenant schemas into pieces of equal parts to then be proccessed in ' - 'parts (requires --of).')) - parser.add_argument('--of', action='store', dest='migration_part_of', - type=greater_than_x(0, 'The number needs to be greated than zero'), default=None, - help='The part you want to process from the pieces (requires --part).') + parser.add_argument('--part', action='store', dest='part', type=greater_than_zero, default=None, + help=('The part you want to process from the pieces (requires --of). ' + 'Example: --part 2 --of 3')) + parser.add_argument('--of', action='store', dest='total_parts', type=greater_than_zero, default=None, + help=('Splits the tenant schemas into specified number of pieces of equal size to ' + 'then be proccessed in parts (requires --part). Example: --part 2 --of 3')) def handle(self, *args, **options): super(Command, self).handle(*args, **options) - required_together = (self.options['migration_part'], self.options['migration_part_of'],) + required_together = (self.options['total_parts'], self.options['part'],) if any(required_together) and not all(required_together): raise Exception("--part and --of need to be used together.") elif all(required_together): - if self.options['migration_part_of'] > self.options['migration_part']: + if self.options['part'] > self.options['total_parts']: raise Exception("--of cannot be greater than --part.") elif self.sync_public: raise Exception("Cannot run public schema migrations along with --of and --part.") @@ -85,7 +89,12 @@ def handle(self, *args, **options): else: tenants = get_tenant_model().objects.exclude(schema_name=get_public_schema_name()) \ .order_by('pk').values_list('schema_name', flat=True) - if self.options['migration_part'] and self.options['migration_part'] > 1 and tenants: - tenant_parts = list(chunks(tenants, self.options['migration_part'] - 1)) - tenants = tenant_parts[self.options['migration_part_of'] - 1] + if self.options['total_parts'] and tenants: + tenant_parts = list(chunks(tenants, self.options['total_parts'])) + try: + tenants = tenant_parts[self.options['part'] - 1] + except IndexError: + message = 'You have fewer tenants than parts. This part (%s) has nothing to do.\n' + self.stdout.write(message % self.options['part']) + return executor.run_migrations(tenants=tenants) From f0bb7377c7f8a2c2916725cead1177309d4b1bed Mon Sep 17 00:00:00 2001 From: buzzi Date: Fri, 22 Jun 2018 18:40:20 +0200 Subject: [PATCH 3/6] No need to check if there is only one part, splicing works properly --- tenant_schemas/management/commands/migrate_schemas.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tenant_schemas/management/commands/migrate_schemas.py b/tenant_schemas/management/commands/migrate_schemas.py index 60ef69fa..5b0d3083 100644 --- a/tenant_schemas/management/commands/migrate_schemas.py +++ b/tenant_schemas/management/commands/migrate_schemas.py @@ -19,9 +19,6 @@ def chunks(tenants, total_parts): """ Iterates over tenants, returning each part, one at a time """ - # import ipdb; print('\a'); ipdb.sset_trace() - if total_parts == 1: - yield tenants tenants_per_chunk = int(math.ceil(float(len(tenants)) / total_parts)) for i in range(0, len(tenants), tenants_per_chunk): yield tenants[i:i + tenants_per_chunk] From 025d96494af02f3cf8e4c99fd26f2590aee366c5 Mon Sep 17 00:00:00 2001 From: buzzi Date: Mon, 9 Jul 2018 18:23:56 +0200 Subject: [PATCH 4/6] Adds tests --- tenant_schemas/tests/test_tenants.py | 122 +++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) diff --git a/tenant_schemas/tests/test_tenants.py b/tenant_schemas/tests/test_tenants.py index 2bf26d8a..774298db 100644 --- a/tenant_schemas/tests/test_tenants.py +++ b/tenant_schemas/tests/test_tenants.py @@ -1,4 +1,5 @@ import json +import argparse try: # python 2 from StringIO import StringIO @@ -6,12 +7,15 @@ # python 3 from io import StringIO +from mock import patch + from django.conf import settings from django.contrib.auth.models import User from django.core.management import call_command from django.db import connection from dts_test_app.models import DummyModel, ModelWithFkToPublicUser +from tenant_schemas.management.commands.migrate_schemas import greater_than_zero from tenant_schemas.test.cases import TenantTestCase from tenant_schemas.tests.models import Tenant, NonAutoSyncTenant from tenant_schemas.tests.testcases import BaseTestCase @@ -372,3 +376,121 @@ def test_tenant_survives_after_method1(self): def test_tenant_survives_after_method2(self): # The same tenant still exists even after the previous method call self.assertEquals(1, get_tenant_model().objects.all().count()) + + +class MigrateSchemasTest(BaseTestCase, TenantTestCase): + + def test_simple_options_command(self): + get_tenant_model = patch('tenant_schemas.management.commands.migrate_schemas.get_tenant_model') + get_executor = patch('tenant_schemas.management.commands.migrate_schemas.get_executor') + + with get_tenant_model, get_executor: + run_migrations = get_executor.return_value.return_value.run_migrations + query_obj = get_tenant_model.return_value.objects.exclude.return_value.order_by.return_value.values_list + query_obj.return_value = ['a', 'b', 'c'] + + call_command('migrate_schemas', + tenant=True, + part=1, + of=3) + + query_obj.assert_called_once() + run_migrations.assert_called_once_with(tenants=['a']) + + call_command('migrate_schemas', + tenant=True, + part=2, + of=3) + + run_migrations.assert_called_with(tenants=['b']) + + call_command('migrate_schemas', + tenant=True, + part=3, + of=3) + + run_migrations.assert_called_with(tenants=['c']) + + with self.assertRaises(Exception) as context: + call_command('migrate_schemas', + tenant=True, + part=4, + of=3) + + self.assertIn('--of cannot be greater than --part.', str(context.exception)) + + def test_rounding_command(self): + get_tenant_model = patch('tenant_schemas.management.commands.migrate_schemas.get_tenant_model') + get_executor = patch('tenant_schemas.management.commands.migrate_schemas.get_executor') + + with get_tenant_model, get_executor: + run_migrations = get_executor.return_value.return_value.run_migrations + query_obj = get_tenant_model.return_value.objects.exclude.return_value.order_by.return_value.values_list + query_obj.return_value = ['a'] + + call_command('migrate_schemas', + tenant=True, + part=1, + of=2) + + run_migrations.assert_called_with(tenants=['a']) + + out = StringIO() + call_command('migrate_schemas', + tenant=True, + part=2, + of=2, + stdout=out) + self.assertIn('You have fewer tenants than parts', out.getvalue()) + + def test_rounding2_command(self): + get_tenant_model = patch('tenant_schemas.management.commands.migrate_schemas.get_tenant_model') + get_executor = patch('tenant_schemas.management.commands.migrate_schemas.get_executor') + + with get_tenant_model, get_executor: + run_migrations = get_executor.return_value.return_value.run_migrations + query_obj = get_tenant_model.return_value.objects.exclude.return_value.order_by.return_value.values_list + query_obj.return_value = list(range(53)) + + call_command('migrate_schemas', + tenant=True, + part=1, + of=7) + + run_migrations.assert_called_with(tenants=[0, 1, 2, 3, 4, 5, 6, 7]) + + call_command('migrate_schemas', + tenant=True, + part=7, + of=7) + + run_migrations.assert_called_with(tenants=[48, 49, 50, 51, 52]) + + def test_errors_command(self): + + with self.assertRaises(Exception) as context: + call_command('migrate_schemas', + part=1, + of=1) + self.assertIn('Cannot run public schema migrations along with --of and --part.', str(context.exception)) + + with self.assertRaises(Exception) as context: + call_command('migrate_schemas', + of=1) + self.assertIn('need to be used together', str(context.exception)) + + greater_than_zero('1') + with self.assertRaises(argparse.ArgumentTypeError) as context: + greater_than_zero('0') + self.assertIn('The number needs to be greated than zero', str(context.exception)) + + with self.assertRaises(argparse.ArgumentTypeError) as context: + greater_than_zero('0a') + self.assertIn('Needs to be a number', str(context.exception)) + + # # Note: Before django 2.0, django bypasses the the "type=" checker in the argparse completely + # with self.assertRaises(Exception) as context: + # call_command('migrate_schemas', + # tenant=True, + # part='a', + # of='a') From da4565795e62240807c93c8f6c3d13b5e01acdec Mon Sep 17 00:00:00 2001 From: buzzi Date: Mon, 9 Jul 2018 18:43:40 +0200 Subject: [PATCH 5/6] Fixes last minute change that broke the tests --- tenant_schemas/tests/test_tenants.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tenant_schemas/tests/test_tenants.py b/tenant_schemas/tests/test_tenants.py index 774298db..eb72662f 100644 --- a/tenant_schemas/tests/test_tenants.py +++ b/tenant_schemas/tests/test_tenants.py @@ -381,10 +381,10 @@ def test_tenant_survives_after_method2(self): class MigrateSchemasTest(BaseTestCase, TenantTestCase): def test_simple_options_command(self): - get_tenant_model = patch('tenant_schemas.management.commands.migrate_schemas.get_tenant_model') - get_executor = patch('tenant_schemas.management.commands.migrate_schemas.get_executor') + get_tenant_model_mock = patch('tenant_schemas.management.commands.migrate_schemas.get_tenant_model') + get_executor_mock = patch('tenant_schemas.management.commands.migrate_schemas.get_executor') - with get_tenant_model, get_executor: + with get_tenant_model_mock as get_tenant_model, get_executor_mock as get_executor: run_migrations = get_executor.return_value.return_value.run_migrations query_obj = get_tenant_model.return_value.objects.exclude.return_value.order_by.return_value.values_list query_obj.return_value = ['a', 'b', 'c'] @@ -420,10 +420,10 @@ def test_simple_options_command(self): self.assertIn('--of cannot be greater than --part.', str(context.exception)) def test_rounding_command(self): - get_tenant_model = patch('tenant_schemas.management.commands.migrate_schemas.get_tenant_model') - get_executor = patch('tenant_schemas.management.commands.migrate_schemas.get_executor') + get_tenant_model_mock = patch('tenant_schemas.management.commands.migrate_schemas.get_tenant_model') + get_executor_mock = patch('tenant_schemas.management.commands.migrate_schemas.get_executor') - with get_tenant_model, get_executor: + with get_tenant_model_mock as get_tenant_model, get_executor_mock as get_executor: run_migrations = get_executor.return_value.return_value.run_migrations query_obj = get_tenant_model.return_value.objects.exclude.return_value.order_by.return_value.values_list query_obj.return_value = ['a'] @@ -444,10 +444,10 @@ def test_rounding_command(self): self.assertIn('You have fewer tenants than parts', out.getvalue()) def test_rounding2_command(self): - get_tenant_model = patch('tenant_schemas.management.commands.migrate_schemas.get_tenant_model') - get_executor = patch('tenant_schemas.management.commands.migrate_schemas.get_executor') + get_tenant_model_mock = patch('tenant_schemas.management.commands.migrate_schemas.get_tenant_model') + get_executor_mock = patch('tenant_schemas.management.commands.migrate_schemas.get_executor') - with get_tenant_model, get_executor: + with get_tenant_model_mock as get_tenant_model, get_executor_mock as get_executor: run_migrations = get_executor.return_value.return_value.run_migrations query_obj = get_tenant_model.return_value.objects.exclude.return_value.order_by.return_value.values_list query_obj.return_value = list(range(53)) From fcc879004871b70bb75d9d041ee456b0d95171bb Mon Sep 17 00:00:00 2001 From: buzzi Date: Mon, 9 Jul 2018 19:55:04 +0200 Subject: [PATCH 6/6] Removes useless code, moves the comments up --- tenant_schemas/tests/test_tenants.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tenant_schemas/tests/test_tenants.py b/tenant_schemas/tests/test_tenants.py index eb72662f..4255c956 100644 --- a/tenant_schemas/tests/test_tenants.py +++ b/tenant_schemas/tests/test_tenants.py @@ -479,6 +479,8 @@ def test_errors_command(self): of=1) self.assertIn('need to be used together', str(context.exception)) + # Note: Before django 2.0, django bypasses the the "type=" checker in the argparse completely, thats why + # we check the underlining function directly. greater_than_zero('1') with self.assertRaises(argparse.ArgumentTypeError) as context: greater_than_zero('0') @@ -487,10 +489,3 @@ def test_errors_command(self): with self.assertRaises(argparse.ArgumentTypeError) as context: greater_than_zero('0a') self.assertIn('Needs to be a number', str(context.exception)) - - # # Note: Before django 2.0, django bypasses the the "type=" checker in the argparse completely - # with self.assertRaises(Exception) as context: - # call_command('migrate_schemas', - # tenant=True, - # part='a', - # of='a')