From 6b1bba6326eec787e34d4ca0fdb839f3dd024469 Mon Sep 17 00:00:00 2001 From: david_wobrock Date: Mon, 4 Sep 2017 17:13:32 +0200 Subject: [PATCH] PEP8 --- migration_linter.py | 164 ++++++++++++++++++++++++++++++++------------ 1 file changed, 122 insertions(+), 42 deletions(-) diff --git a/migration_linter.py b/migration_linter.py index a35376c..9e755c6 100644 --- a/migration_linter.py +++ b/migration_linter.py @@ -23,20 +23,26 @@ import sys def has_default(sql, **kwargs): if re.search('SET DEFAULT', sql) and kwargs['errors']: - err = next(err for err in kwargs['errors'] if err['code'] == 'NOT_NULL') + err = next(err + for err in kwargs['errors'] + if err['code'] == 'NOT_NULL') if err: - log.info('Found a NOT_NULL error in migration, but it has a default value added: {}'.format(err)) + log.info( + ('Found a NOT_NULL error in migration, ' + 'but it has a default value added: {}').format(err)) kwargs['errors'].remove(err) return False # Never fails + class MigrationLinter: MIGRATION_FOLDER_NAME = 'migrations' migration_tests = ( { 'code': 'NOT_NULL', - 'fn': lambda sql, **kw: re.search('NOT NULL', sql) and not re.search('CREATE TABLE', sql), + 'fn': lambda sql, **kw: re.search('NOT NULL', sql) and + not re.search('CREATE TABLE', sql), 'err_msg': 'NOT NULL constraint on columns' }, { 'code': 'DROP_COLUMN', @@ -44,11 +50,13 @@ class MigrationLinter: 'err_msg': 'DROPPING columns' }, { 'code': 'RENAME_COLUMN', - 'fn': lambda sql, **kw: re.search('ALTER TABLE .* CHANGE', sql) or re.search('ALTER TABLE .* RENAME COLUMN', sql), # mysql or postgre + 'fn': lambda sql, **kw: re.search('ALTER TABLE .* CHANGE', sql) or + re.search('ALTER TABLE .* RENAME COLUMN', sql), 'err_msg': 'RENAMING columns' }, { 'code': 'RENAME_TABLE', - 'fn': lambda sql, **kw: re.search('RENAME TABLE', sql) or re.search('ALTER TABLE .* RENAME TO', sql), # mysql or postgre + 'fn': lambda sql, **kw: re.search('RENAME TABLE', sql) or + re.search('ALTER TABLE .* RENAME TO', sql), 'err_msg': 'RENAMING tables' }, { 'code': '', @@ -72,7 +80,8 @@ class MigrationLinter: def _gather_migrations(self): # Find (one of) the initial commits if not self.commit_id: - git_init_cmd = 'cd {0} && git rev-list HEAD | tail -n 1'.format(self.location) + git_init_cmd = 'cd {0} && git rev-list HEAD | tail -n 1'.format( + self.location) process = Popen(git_init_cmd, shell=True, stdout=PIPE, stderr=PIPE) for line in process.stdout.readlines(): self.commit_id = line.strip() @@ -81,19 +90,25 @@ class MigrationLinter: log.info('Operating until git identifier {0}'.format(self.commit_id)) # Get changes since specified commit - git_diff_command = 'cd {0} && git diff --name-only {1}'.format(self.location, self.commit_id) + git_diff_command = 'cd {0} && git diff --name-only {1}'.format( + self.location, self.commit_id) log.info('Executing {0}'.format(git_diff_command)) - diff_process = Popen(git_diff_command, shell=True, stdout=PIPE, stderr=PIPE) + diff_process = Popen( + git_diff_command, shell=True, stdout=PIPE, stderr=PIPE) for line in diff_process.stdout.readlines(): # Only gather lines that include migrations - if re.search('\/{0}\/.*\.py'.format(self.MIGRATION_FOLDER_NAME), line) and '__init__' not in line: + if re.search( + '\/{0}\/.*\.py'.format(self.MIGRATION_FOLDER_NAME), + line) and \ + '__init__' not in line: self.changed_migration_files.append(line.strip()) diff_process.wait() if diff_process.returncode != 0: output = [] for line in diff_process.stderr.readlines(): output.append(line) - log.info("Error while git diff command:\n{}".format("".join(output))) + log.info("Error while git diff command:\n{}".format( + "".join(output))) raise Exception('Error while executing git diff command') def lint_migrations(self): @@ -107,10 +122,13 @@ class MigrationLinter: print('IGNORE') nb_ignore += 1 else: - sql_statements = self.django_sqlmigrate(app_name, migration_name) + sql_statements = self.django_sqlmigrate( + app_name, migration_name) errors = [] for statement in sql_statements: - is_valid, err = self._test_sql_statement_for_backward_incompatibility(statement, errors) + is_valid, err = \ + self._test_sql_statement_for_backward_incompatibility( + statement, errors) if not is_valid: errors.append(err) if not errors: @@ -124,11 +142,18 @@ class MigrationLinter: if err['table']: error_str += ' (table: {0}'.format(err['table']) if err['column']: - error_str += ', column: {0}'.format(err['column']) + error_str += ', column: {0}'.format( + err['column']) error_str += ')' print(error_str) print('*** Summary:') - print('Valid migrations: {0}/{1} - erroneous migrations: {2}/{1} - ignored migrations: {3}/{1}'.format(nb_valid, len(self.changed_migration_files), nb_erroneous, nb_ignore)) + print(('Valid migrations: {0}/{1} - ' + 'erroneous migrations: {2}/{1} - ' + 'ignored migrations: {3}/{1}').format( + nb_valid, + len(self.changed_migration_files), + nb_erroneous, + nb_ignore)) return nb_erroneous > 0 @classmethod @@ -136,21 +161,30 @@ class MigrationLinter: decomposed_path = split_path(migration_path) for i, p in enumerate(decomposed_path): if p == cls.MIGRATION_FOLDER_NAME: - return decomposed_path[i-1], os.path.splitext(decomposed_path[i+1])[0] + return (decomposed_path[i-1], + os.path.splitext(decomposed_path[i+1])[0]) def should_ignore_migration(self, app_name, migration_name): - return (self.include_apps and app_name not in self.include_apps)\ - or (self.exclude_apps and app_name in self.exclude_apps)\ - or (self.ignore_name_contains and self.ignore_name_contains in migration_name)\ + return (self.include_apps and + app_name not in self.include_apps)\ + or (self.exclude_apps and + app_name in self.exclude_apps)\ + or (self.ignore_name_contains and + self.ignore_name_contains in migration_name)\ or (migration_name in self.ignore_name) def django_sqlmigrate(self, app_name, migration_name): - git_diff_command = 'cd {0} && python manage.py sqlmigrate {1} {2} --database {3}'.format(self.location, app_name, migration_name, self.database) + git_diff_command = ( + 'cd {0} && ' + 'python manage.py sqlmigrate {1} {2} ' + '--database {3}').format( + self.location, app_name, migration_name, self.database) log.info('Executing {0}'.format(git_diff_command)) - diff_process = Popen(git_diff_command, shell=True, stdout=PIPE, stderr=PIPE) + diff_process = Popen( + git_diff_command, shell=True, stdout=PIPE, stderr=PIPE) sql_statements = [] for line in diff_process.stdout.readlines(): - if not line.startswith('--'): # Do not take sql comments into account + if not line.startswith('--'): # Ignore comments sql_statements.append(line.strip()) diff_process.wait() if diff_process.returncode != 0: @@ -158,12 +192,15 @@ class MigrationLinter: log.info('Found {0} sql migration lines'.format(len(sql_statements))) return sql_statements - def _test_sql_statement_for_backward_incompatibility(self, sql_statement, errors): + def _test_sql_statement_for_backward_incompatibility( + self, sql_statement, errors): for test in self.migration_tests: if test['fn'](sql_statement, errors=errors): log.info('Testing {0} -- ERROR'.format(sql_statement)) - table_search = re.search('TABLE `([^`]*)`', sql_statement, re.IGNORECASE) - col_search = re.search('COLUMN `([^`]*)`', sql_statement, re.IGNORECASE) + table_search = re.search( + 'TABLE `([^`]*)`', sql_statement, re.IGNORECASE) + col_search = re.search( + 'COLUMN `([^`]*)`', sql_statement, re.IGNORECASE) err = { 'err_msg': test['err_msg'], 'code': test['code'], @@ -185,11 +222,13 @@ def valid_folder(folder): return False django_manage_file = os.path.join(folder, 'manage.py') if not os.path.isfile(django_manage_file): - print("The passed folder doesn't seem to be a django project (no manage.py found).") + print(("The passed folder doesn't seem to be a " + "django project (no manage.py found).")) return False git_folder = os.path.join(folder, '.git') if not os.path.isdir(git_folder): - print("The passed folder doesn't seem to versioned by git (no .git/ folder found).") + print(("The passed folder doesn't seem to be " + "versioned by git (no .git/ folder found).")) return False return True @@ -200,16 +239,55 @@ def split_path(path): if __name__ == '__main__': - parser = argparse.ArgumentParser(description='Detect backward incompatible django migrations.') - parser.add_argument('django_folder', metavar='DJANGO_FOLDER', type=str, nargs=1, help='the path to the django project') - parser.add_argument('commit_id', metavar='GIT_COMMIT_ID', type=str, nargs='?', help='if specified, only migrations since this commit will be taken into account. If not specified, the initial repo commit will be used') - parser.add_argument('--ignore-name-contains', type=str, nargs='?', help='ignore migrations containing this name') - parser.add_argument('--ignore-name', type=str, nargs='*', help='ignore migrations with exactly one of these names') - parser.add_argument('--verbose', '-v', action='store_true', help='print more information during execution') - parser.add_argument('--database', type=str, nargs='?', help='specify the database for which to generate the SQL. Defaults to default') - incl_excl_group = parser.add_mutually_exclusive_group(required=False) - incl_excl_group.add_argument('--include-apps', type=str, nargs='*', help='check only migrations that are in the specified django apps') - incl_excl_group.add_argument('--exclude-apps', type=str, nargs='*', help='ignore migrations that are in the specified django apps') + parser = argparse.ArgumentParser( + description='Detect backward incompatible django migrations.') + parser.add_argument( + 'django_folder', + metavar='DJANGO_FOLDER', + type=str, + nargs=1, + help='the path to the django project') + parser.add_argument( + 'commit_id', + metavar='GIT_COMMIT_ID', + type=str, nargs='?', + help=('if specified, only migrations since this commit ' + 'will be taken into account. If not specified, ' + 'the initial repo commit will be used')) + parser.add_argument( + '--ignore-name-contains', + type=str, + nargs='?', + help='ignore migrations containing this name') + parser.add_argument( + '--ignore-name', + type=str, + nargs='*', + help='ignore migrations with exactly one of these names') + parser.add_argument( + '--verbose', + '-v', + action='store_true', + help='print more information during execution') + parser.add_argument( + '--database', + type=str, + nargs='?', + help=('specify the database for which to generate the SQL. ' + 'Defaults to default')) + + incl_excl_group = parser.add_mutually_exclusive_group( + required=False) + incl_excl_group.add_argument( + '--include-apps', + type=str, + nargs='*', + help='check only migrations that are in the specified django apps') + incl_excl_group.add_argument( + '--exclude-apps', + type=str, + nargs='*', + help='ignore migrations that are in the specified django apps') args = parser.parse_args() if args.verbose: @@ -219,12 +297,14 @@ if __name__ == '__main__': folder_name = args.django_folder[0] if valid_folder(folder_name): - linter = MigrationLinter(folder_name, args.commit_id, - ignore_name_contains=args.ignore_name_contains, - ignore_name=args.ignore_name, - include_apps=args.include_apps, - exclude_apps=args.exclude_apps, - database=args.database) + linter = MigrationLinter( + folder_name, + args.commit_id, + ignore_name_contains=args.ignore_name_contains, + ignore_name=args.ignore_name, + include_apps=args.include_apps, + exclude_apps=args.exclude_apps, + database=args.database) has_errors = linter.lint_migrations() if has_errors: sys.exit(1)