diff --git a/awx/api/views.py b/awx/api/views.py index 75f523cdf6..97a3d57400 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -2663,12 +2663,6 @@ class InventoryUpdateList(ListAPIView): model = InventoryUpdate serializer_class = InventoryUpdateListSerializer - def get_queryset(self): - qs = super(InventoryUpdateList, self).get_queryset() - # TODO: remove this defer in 3.3 when we implement https://github.com/ansible/ansible-tower/issues/5436 - qs = qs.defer('result_stdout_text') - return qs - class InventoryUpdateDetail(UnifiedJobDeletionMixin, RetrieveDestroyAPIView): diff --git a/awx/main/access.py b/awx/main/access.py index e38c85d984..105b925577 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -140,14 +140,7 @@ def get_user_capabilities(user, instance, **kwargs): convenient for the user interface to consume and hide or show various actions in the interface. ''' - cls = instance.__class__ - # When `.defer()` is used w/ the Django ORM, the result is a subclass of - # the original that represents e.g., - # awx.main.models.ad_hoc_commands.AdHocCommand_Deferred_result_stdout_text - # We want to do the access registry lookup keyed on the base class name. - if getattr(cls, '_deferred', False): - cls = instance.__class__.__bases__[0] - access_class = access_registry[cls] + access_class = access_registry[instance.__class__] return access_class(user).get_user_capabilities(instance, **kwargs) @@ -2075,8 +2068,7 @@ class UnifiedJobAccess(BaseAccess): Q(job__inventory__organization__in=org_auditor_qs) | Q(job__project__organization__in=org_auditor_qs) ) - # TODO: remove this defer in 3.3 when we implement https://github.com/ansible/ansible-tower/issues/5436 - return qs.defer('result_stdout_text') + return qs class ScheduleAccess(BaseAccess): diff --git a/awx/main/migrations/0013_move_deprecated_stdout.py b/awx/main/migrations/0013_move_deprecated_stdout.py new file mode 100644 index 0000000000..f2c6250649 --- /dev/null +++ b/awx/main/migrations/0013_move_deprecated_stdout.py @@ -0,0 +1,44 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.7 on 2017-12-12 18:56 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0012_non_blank_workflow'), + ] + + operations = [ + migrations.AlterField( + model_name='unifiedjob', + name='result_stdout_text', + field=models.TextField(editable=False, null=True), + ), + # Using SeparateDatabaseAndState here allows us to update the migration + # state so that Django thinks the UnifiedJob.result_stdout_text field + # is gone _without_ actually deleting the underlying column/data + migrations.SeparateDatabaseAndState(state_operations=[ + migrations.RemoveField( + model_name='unifiedjob', + name='result_stdout_text', + ), + ]), + # On other side of the equation, this migration introduces a new model + # which is *unmanaged* (meaning, a new table is not created for it); + # instead, this sort of "virtual" model is used to maintain an ORM + # reference to the actual `main_unifiedjob.result_stdout_text` column + migrations.CreateModel( + name='UnifiedJobDeprecatedStdout', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('result_stdout_text', models.TextField(editable=False, null=True)) + ], + options={ + 'db_table': 'main_unifiedjob', + 'managed': False, + }, + ), + ] diff --git a/awx/main/models/__init__.py b/awx/main/models/__init__.py index 66aab8242f..01bd68cc66 100644 --- a/awx/main/models/__init__.py +++ b/awx/main/models/__init__.py @@ -145,17 +145,3 @@ activity_stream_registrar.connect(WorkflowJob) # prevent API filtering on certain Django-supplied sensitive fields prevent_search(User._meta.get_field('password')) - - -# Always, always, always defer result_stdout_text for polymorphic UnifiedJob rows -# TODO: remove this defer in 3.3 when we implement https://github.com/ansible/ansible-tower/issues/5436 -def defer_stdout(f): - def _wrapped(*args, **kwargs): - objs = f(*args, **kwargs) - objs.query.deferred_loading[0].add('result_stdout_text') - return objs - return _wrapped - - -for cls in UnifiedJob.__subclasses__(): - cls.base_objects.filter = defer_stdout(cls.base_objects.filter) diff --git a/awx/main/models/unified_jobs.py b/awx/main/models/unified_jobs.py index a28ebff4d4..235b048660 100644 --- a/awx/main/models/unified_jobs.py +++ b/awx/main/models/unified_jobs.py @@ -492,6 +492,18 @@ class UnifiedJobTypeStringMixin(object): return UnifiedJobTypeStringMixin._camel_to_underscore(self.__class__.__name__) +class UnifiedJobDeprecatedStdout(models.Model): + + class Meta: + managed = False + db_table = 'main_unifiedjob' + + result_stdout_text = models.TextField( + null=True, + editable=False, + ) + + class UnifiedJob(PolymorphicModel, PasswordFieldsModel, CommonModelNameNotUnique, UnifiedJobTypeStringMixin, TaskManagerUnifiedJobMixin): ''' Concrete base class for unified job run by the task engine. @@ -620,11 +632,6 @@ class UnifiedJob(PolymorphicModel, PasswordFieldsModel, CommonModelNameNotUnique default='', editable=False, )) - result_stdout_text = models.TextField( - blank=True, - default='', - editable=False, - ) result_stdout_file = models.TextField( # FilePathfield? blank=True, default='', @@ -882,6 +889,19 @@ class UnifiedJob(PolymorphicModel, PasswordFieldsModel, CommonModelNameNotUnique config.credentials.add(*job_creds) return config + @property + def result_stdout_text(self): + related = UnifiedJobDeprecatedStdout.objects.get(pk=self.pk) + return related.result_stdout_text or '' + + @result_stdout_text.setter + def result_stdout_text(self, value): + # TODO: remove this method once all stdout is based on jobevents + # (because it won't be used for writing anymore) + related = UnifiedJobDeprecatedStdout.objects.get(pk=self.pk) + related.result_stdout_text = value + related.save() + def result_stdout_raw_handle(self, attempt=0): """Return a file-like object containing the standard out of the job's result. diff --git a/awx/main/tasks.py b/awx/main/tasks.py index b9fe68908d..ac33b497cc 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -516,6 +516,10 @@ class BaseTask(LogErrorsTask): update_fields.append(field) if field == 'status': update_fields.append('failed') + if 'result_stdout_text' in update_fields: + # result_stdout_text is now deprecated, and is no longer + # an actual Django field (it's a property) + update_fields.remove('result_stdout_text') instance.save(update_fields=update_fields) return instance except DatabaseError as e: diff --git a/awx/main/tests/functional/__init__.py b/awx/main/tests/functional/__init__.py index e69de29bb2..405018d1a4 100644 --- a/awx/main/tests/functional/__init__.py +++ b/awx/main/tests/functional/__init__.py @@ -0,0 +1,23 @@ +from django.db import connection +from django.db.models.signals import post_migrate +from django.apps import apps + + +def app_post_migration(sender, app_config, **kwargs): + # our usage of pytest.django+sqlite doesn't actually run real migrations, + # so we've got to make sure the deprecated + # `main_unifiedjob.result_stdout_text` column actually exists + cur = connection.cursor() + cols = cur.execute( + 'SELECT sql FROM sqlite_master WHERE tbl_name="main_unifiedjob";' + ).fetchone()[0] + if 'result_stdout_text' not in cols: + cur.execute( + 'ALTER TABLE main_unifiedjob ADD COLUMN result_stdout_text TEXT' + ) + + +post_migrate.connect(app_post_migration, sender=apps.get_app_config('main')) + + + diff --git a/awx/main/tests/functional/api/test_unified_jobs_view.py b/awx/main/tests/functional/api/test_unified_jobs_view.py index 1baaa932da..c537dd941f 100644 --- a/awx/main/tests/functional/api/test_unified_jobs_view.py +++ b/awx/main/tests/functional/api/test_unified_jobs_view.py @@ -33,7 +33,9 @@ TEST_STDOUTS.append({ def test_cases(project): ret = [] for e in TEST_STDOUTS: - e['project'] = ProjectUpdate(project=project) + pu = ProjectUpdate(project=project) + pu.save() + e['project'] = pu e['project'].result_stdout_text = e['text'] e['project'].save() ret.append(e) diff --git a/awx/main/tests/unit/test_unified_jobs.py b/awx/main/tests/unit/test_unified_jobs.py index 592c4783b1..659eed63a5 100644 --- a/awx/main/tests/unit/test_unified_jobs.py +++ b/awx/main/tests/unit/test_unified_jobs.py @@ -8,15 +8,15 @@ from StringIO import StringIO from django.utils.timezone import now # AWX -from awx.main.models import UnifiedJob +from awx.main import models # stdout file present @mock.patch('os.path.exists', return_value=True) @mock.patch('codecs.open', return_value='my_file_handler') +@mock.patch.object(models.UnifiedJob, 'result_stdout_text', '') def test_result_stdout_raw_handle_file__found(exists, open): - unified_job = UnifiedJob() - unified_job.result_stdout_file = 'dummy' + unified_job = models.UnifiedJob() with mock.patch('os.stat', return_value=Mock(st_size=1)): result = unified_job.result_stdout_raw_handle() @@ -26,8 +26,9 @@ def test_result_stdout_raw_handle_file__found(exists, open): # stdout file missing, job finished @mock.patch('os.path.exists', return_value=False) +@mock.patch.object(models.UnifiedJob, 'result_stdout_text', '') def test_result_stdout_raw_handle__missing(exists): - unified_job = UnifiedJob() + unified_job = models.UnifiedJob() unified_job.result_stdout_file = 'dummy' unified_job.finished = now() @@ -39,8 +40,9 @@ def test_result_stdout_raw_handle__missing(exists): # stdout file missing, job not finished @mock.patch('os.path.exists', return_value=False) +@mock.patch.object(models.UnifiedJob, 'result_stdout_text', '') def test_result_stdout_raw_handle__pending(exists): - unified_job = UnifiedJob() + unified_job = models.UnifiedJob() unified_job.result_stdout_file = 'dummy' unified_job.finished = None