From 76a55dbb128f6f4cddc320624d42f3e67ea4c9bd Mon Sep 17 00:00:00 2001 From: David Wobrock Date: Fri, 28 Jan 2022 22:05:00 +0100 Subject: [PATCH] Raise when unsupported db vendor, and add option to specify SQL analyser --- CHANGELOG.md | 1 + .../management/commands/lintmigrations.py | 8 ++++ django_migration_linter/migration_linter.py | 13 ++++-- .../sql_analyser/__init__.py | 2 +- .../sql_analyser/analyser.py | 31 +++++++++++--- docs/usage.md | 41 ++++++++++--------- tests/unit/test_sql_analyser.py | 8 +++- 7 files changed, 71 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05566f8..371fb1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Fix index creation detection when table is being created in the transaction (issue #178) - Handle unique index creation as adding a unique constraint (issue #183) - Allow any option to be set/unset in config file (issue #167) +- Raise when unsupported database vendor, allow passing an option to select SQL analyser (issue #138 and #169) ## 3.0.1 diff --git a/django_migration_linter/management/commands/lintmigrations.py b/django_migration_linter/management/commands/lintmigrations.py index 61de97c..39d7d8e 100644 --- a/django_migration_linter/management/commands/lintmigrations.py +++ b/django_migration_linter/management/commands/lintmigrations.py @@ -10,6 +10,7 @@ from django.core.management.base import BaseCommand from ...constants import __version__ from ...migration_linter import MessageType, MigrationLinter +from ...sql_analyser.analyser import ANALYSER_STRING_MAPPING from ..utils import register_linting_configuration_options CONFIG_NAME = "django_migration_linter" @@ -127,6 +128,12 @@ class Command(BaseCommand): choices=MessageType.values(), help="don't print linting messages to stdout", ) + parser.add_argument( + "--sql-analyser", + nargs="?", + choices=list(ANALYSER_STRING_MAPPING.keys()), + help="select the SQL analyser", + ) register_linting_configuration_options(parser) def handle(self, *args, **options): @@ -167,6 +174,7 @@ class Command(BaseCommand): quiet=options["quiet"], warnings_as_errors=options["warnings_as_errors"], no_output=options["verbosity"] == 0, + analyser_string=options["sql_analyser"], ) linter.lint_all_migrations( app_label=options["app_label"], diff --git a/django_migration_linter/migration_linter.py b/django_migration_linter/migration_linter.py index be9c992..e9f4fef 100644 --- a/django_migration_linter/migration_linter.py +++ b/django_migration_linter/migration_linter.py @@ -20,7 +20,7 @@ from .constants import ( EXPECTED_DATA_MIGRATION_ARGS, ) from .operations import IgnoreMigration -from .sql_analyser import analyse_sql_statements +from .sql_analyser import analyse_sql_statements, get_sql_analyser_class from .utils import clean_bytes_to_str, get_migration_abspath, split_migration_path logger = logging.getLogger("django_migration_linter") @@ -57,6 +57,7 @@ class MigrationLinter(object): quiet=None, warnings_as_errors=False, no_output=False, + analyser_string=None, ): # Store parameters and options self.django_path = path @@ -75,6 +76,10 @@ class MigrationLinter(object): self.quiet = quiet or [] self.warnings_as_errors = warnings_as_errors self.no_output = no_output + self.sql_analyser_class = get_sql_analyser_class( + settings.DATABASES[self.database]["ENGINE"], + analyser_string=analyser_string, + ) # Initialise counters self.reset_counters() @@ -161,8 +166,8 @@ class MigrationLinter(object): sql_statements = self.get_sql(app_label, migration_name) errors, ignored, warnings = analyse_sql_statements( + self.sql_analyser_class, sql_statements, - settings.DATABASES[self.database]["ENGINE"], self.exclude_migration_tests, ) @@ -620,8 +625,8 @@ class MigrationLinter(object): sql_statements.append(runsql.sql) sql_errors, sql_ignored, sql_warnings = analyse_sql_statements( + self.sql_analyser_class, sql_statements, - settings.DATABASES[self.database]["ENGINE"], self.exclude_migration_tests, ) if sql_errors: @@ -650,8 +655,8 @@ class MigrationLinter(object): sql_statements.append(runsql.reverse_sql) sql_errors, sql_ignored, sql_warnings = analyse_sql_statements( + self.sql_analyser_class, sql_statements, - settings.DATABASES[self.database]["ENGINE"], self.exclude_migration_tests, ) if sql_errors: diff --git a/django_migration_linter/sql_analyser/__init__.py b/django_migration_linter/sql_analyser/__init__.py index 93515c1..ad16b0d 100644 --- a/django_migration_linter/sql_analyser/__init__.py +++ b/django_migration_linter/sql_analyser/__init__.py @@ -3,4 +3,4 @@ from .mysql import MySqlAnalyser # noqa from .postgresql import PostgresqlAnalyser # noqa from .sqlite import SqliteAnalyser # noqa -from .analyser import analyse_sql_statements # noqa isort:skip +from .analyser import analyse_sql_statements, get_sql_analyser_class # noqa isort:skip diff --git a/django_migration_linter/sql_analyser/analyser.py b/django_migration_linter/sql_analyser/analyser.py index d98610a..db259ff 100644 --- a/django_migration_linter/sql_analyser/analyser.py +++ b/django_migration_linter/sql_analyser/analyser.py @@ -1,7 +1,6 @@ import logging from django_migration_linter.sql_analyser import ( - BaseAnalyser, MySqlAnalyser, PostgresqlAnalyser, SqliteAnalyser, @@ -9,8 +8,28 @@ from django_migration_linter.sql_analyser import ( logger = logging.getLogger("django_migration_linter") +ANALYSER_STRING_MAPPING = { + "sqlite": SqliteAnalyser, + "mysql": MySqlAnalyser, + "postgresql": PostgresqlAnalyser, +} -def get_sql_analyser(database_vendor, exclude_migration_tests=None): + +def get_sql_analyser_class(database_vendor, analyser_string=None): + if analyser_string: + return get_sql_analyser_from_string(analyser_string) + return get_sql_analyser_class_from_db_vendor(database_vendor) + + +def get_sql_analyser_from_string(analyser_string): + if analyser_string not in ANALYSER_STRING_MAPPING: + raise ValueError( + "Unknown SQL analyser. Known values: {}".format(analyser_string.keys()) + ) + return ANALYSER_STRING_MAPPING[analyser_string] + + +def get_sql_analyser_class_from_db_vendor(database_vendor): if "mysql" in database_vendor: sql_analyser_class = MySqlAnalyser elif "postgre" in database_vendor: @@ -18,15 +37,15 @@ def get_sql_analyser(database_vendor, exclude_migration_tests=None): elif "sqlite" in database_vendor: sql_analyser_class = SqliteAnalyser else: - sql_analyser_class = BaseAnalyser + raise ValueError("Unsupported database vendor. Try specifying an SQL analyser.") logger.debug("Chosen SQL analyser class: %s", sql_analyser_class) - return sql_analyser_class(exclude_migration_tests) + return sql_analyser_class def analyse_sql_statements( - sql_statements, database_vendor, exclude_migration_tests=None + sql_analyser_class, sql_statements, exclude_migration_tests=None ): - sql_analyser = get_sql_analyser(database_vendor, exclude_migration_tests) + sql_analyser = sql_analyser_class(exclude_migration_tests) sql_analyser.analyse(sql_statements) return sql_analyser.errors, sql_analyser.ignored, sql_analyser.warnings diff --git a/docs/usage.md b/docs/usage.md index 9fd7162..7686e9d 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -19,26 +19,27 @@ The three main usages are: Below the detailed command line options, which can all also be defined using a config file (`setup.cfg`, `tox.ini`, `pyproject.toml`, `.django_migration_linter.cfg`): -| Parameter | Description | -|--------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------| -|`--git-commit-id GIT_COMMIT_ID` | If specified, only migrations since this commit will be taken into account. | -|`--ignore-name-contains IGNORE_NAME_CONTAINS` | Ignore migrations containing this name. | -|`--ignore-name IGNORE_NAME [IGNORE_NAME ...]` | Ignore migrations with exactly one of these names. | -|`--include-name-contains INCLUDE_NAME_CONTAINS` | Include migrations containing this name. | -|`--include-name INCLUDE_NAME [INCLUDE_NAME ...]` | Include migrations with exactly one of these names. | -|`--include-apps INCLUDE_APPS [INCLUDE_APPS ...]` | Check only migrations that are in the specified django apps. | -|`--exclude-apps EXCLUDE_APPS [EXCLUDE_APPS ...]` | Ignore migrations that are in the specified django apps. | -|`--exclude-migration-tests MIGRATION_TEST_CODE [...]` | Specify backward incompatible migration tests to be ignored using the code (e.g. ALTER_COLUMN). | -|`--verbosity or -v {0,1,2,3}` | Print more information during execution. | -|`--database DATABASE` | Specify the database for which to generate the SQL. Defaults to *default*. | -|`--cache-path PATH` | specify a directory that should be used to store cache-files in. | -|`--no-cache` | Don't use a cache. | -|`--applied-migrations` | Only lint migrations that are applied to the selected database. Other migrations are ignored. | -|`--unapplied-migrations` | Only lint migrations that are not yet applied to the selected database. Other migrations are ignored. | -|`--project-root-path DJANGO_PROJECT_FOLDER` | An absolute or relative path to the django project. | -|`--include-migrations-from FILE_PATH` | If specified, only migrations listed in the given file will be considered. | -|`--quiet or -q {ok,ignore,warning,error}` | Suppress certain output messages, instead of writing them to stdout. | -|`--warnings-as-errors` | Handle warnings as errors and therefore return an error status code if we should. | +| Parameter | Description | +|-------------------------------------------------------|-------------------------------------------------------------------------------------------------------| +| `--git-commit-id GIT_COMMIT_ID` | If specified, only migrations since this commit will be taken into account. | +| `--ignore-name-contains IGNORE_NAME_CONTAINS` | Ignore migrations containing this name. | +| `--ignore-name IGNORE_NAME [IGNORE_NAME ...]` | Ignore migrations with exactly one of these names. | +| `--include-name-contains INCLUDE_NAME_CONTAINS` | Include migrations containing this name. | +| `--include-name INCLUDE_NAME [INCLUDE_NAME ...]` | Include migrations with exactly one of these names. | +| `--include-apps INCLUDE_APPS [INCLUDE_APPS ...]` | Check only migrations that are in the specified django apps. | +| `--exclude-apps EXCLUDE_APPS [EXCLUDE_APPS ...]` | Ignore migrations that are in the specified django apps. | +| `--exclude-migration-tests MIGRATION_TEST_CODE [...]` | Specify backward incompatible migration tests to be ignored using the code (e.g. ALTER_COLUMN). | +| `--verbosity or -v {0,1,2,3}` | Print more information during execution. | +| `--database DATABASE` | Specify the database for which to generate the SQL. Defaults to *default*. | +| `--cache-path PATH` | specify a directory that should be used to store cache-files in. | +| `--no-cache` | Don't use a cache. | +| `--applied-migrations` | Only lint migrations that are applied to the selected database. Other migrations are ignored. | +| `--unapplied-migrations` | Only lint migrations that are not yet applied to the selected database. Other migrations are ignored. | +| `--project-root-path DJANGO_PROJECT_FOLDER` | An absolute or relative path to the django project. | +| `--include-migrations-from FILE_PATH` | If specified, only migrations listed in the given file will be considered. | +| `--quiet or -q {ok,ignore,warning,error}` | Suppress certain output messages, instead of writing them to stdout. | +| `--warnings-as-errors` | Handle warnings as errors and therefore return an error status code if we should. | +| `--sql-analyser` | Specify the SQL analyser that should be used. Allowed values: 'sqlite', 'mysql', 'postgresql'. | ## File configuration diff --git a/tests/unit/test_sql_analyser.py b/tests/unit/test_sql_analyser.py index 00929b2..76ca9f6 100644 --- a/tests/unit/test_sql_analyser.py +++ b/tests/unit/test_sql_analyser.py @@ -1,6 +1,9 @@ import unittest -from django_migration_linter.sql_analyser import analyse_sql_statements +from django_migration_linter.sql_analyser import ( + analyse_sql_statements, + get_sql_analyser_class, +) class SqlAnalyserTestCase(unittest.TestCase): @@ -10,7 +13,8 @@ class SqlAnalyserTestCase(unittest.TestCase): if isinstance(sql, str): sql = sql.splitlines() return analyse_sql_statements( - sql_statements=sql, database_vendor=self.database_vendor + get_sql_analyser_class(self.database_vendor), + sql_statements=sql, ) def assertValidSql(self, sql, allow_warnings=False):