Merge pull request ansible#795 from ryanpetrello/move-deprecated-stdout

move legacy UnifiedJob stdout data to a separate unmanaged model

Author: ryanpetrello

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):
 

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 @@ def filtered_queryset(self):
             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):

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,
+            },
+        ),
+    ]

awx/main/models/__init__.py

@@ -145,17 +145,3 @@ def user_is_in_enterprise_category(user, category):
 
 # 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)

awx/main/models/unified_jobs.py

@@ -492,6 +492,18 @@ def model_to_str(self):
         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 Meta:
         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 @@ def create_config_from_prompts(self, kwargs):
             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.

awx/main/tasks.py

@@ -516,6 +516,10 @@ def update_model(self, pk, _attempt=0, **updates):
                         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:

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'))
+
+
+

awx/main/tests/functional/api/test_unified_jobs_view.py

@@ -33,7 +33,9 @@
 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)

awx/main/tests/unit/test_unified_jobs.py

@@ -8,15 +8,15 @@
 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