From 54f99749eb2229dcd0abe62c61b81a4420075e1a Mon Sep 17 00:00:00 2001 From: Aaron Tan Date: Fri, 25 Nov 2016 21:23:29 -0500 Subject: [PATCH 1/8] Provide linkage from spawned job back to wfj. --- awx/api/serializers.py | 2 +- .../0053_v310_linkage_back_to_workflow_job.py | 20 +++++++++++++++++++ awx/main/models/unified_jobs.py | 9 ++++++++- awx/main/scheduler/__init__.py | 5 +++++ 4 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 awx/main/migrations/0053_v310_linkage_back_to_workflow_job.py diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 3103703039..9677c3a62b 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -566,7 +566,7 @@ class UnifiedJobSerializer(BaseSerializer): fields = ('*', 'unified_job_template', 'launch_type', 'status', 'failed', 'started', 'finished', 'elapsed', 'job_args', 'job_cwd', 'job_env', 'job_explanation', 'result_stdout', - 'execution_node', 'result_traceback') + 'execution_node', 'result_traceback', 'source_workflow_job') extra_kwargs = { 'unified_job_template': { 'source': 'unified_job_template_id', diff --git a/awx/main/migrations/0053_v310_linkage_back_to_workflow_job.py b/awx/main/migrations/0053_v310_linkage_back_to_workflow_job.py new file mode 100644 index 0000000000..f98840c3e4 --- /dev/null +++ b/awx/main/migrations/0053_v310_linkage_back_to_workflow_job.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0052_v310_inventory_name_non_unique'), + ] + + operations = [ + migrations.AddField( + model_name='unifiedjob', + name='source_workflow_job', + field=models.ForeignKey(related_name='spawned_jobs', on_delete=django.db.models.deletion.SET_NULL, default=None, editable=False, to='main.WorkflowJob', null=True), + ), + ] diff --git a/awx/main/models/unified_jobs.py b/awx/main/models/unified_jobs.py index 7e95e5abd7..92903a376e 100644 --- a/awx/main/models/unified_jobs.py +++ b/awx/main/models/unified_jobs.py @@ -528,7 +528,14 @@ class UnifiedJob(PolymorphicModel, PasswordFieldsModel, CommonModelNameNotUnique blank=True, related_name='%(class)s_labels' ) - + source_workflow_job = models.ForeignKey( + 'WorkflowJob', + null=True, + default=None, + editable=False, + related_name='spawned_jobs', + on_delete=models.SET_NULL, + ) def get_absolute_url(self): real_instance = self.get_real_instance() diff --git a/awx/main/scheduler/__init__.py b/awx/main/scheduler/__init__.py index 0ac737c375..ba4aa5eed5 100644 --- a/awx/main/scheduler/__init__.py +++ b/awx/main/scheduler/__init__.py @@ -116,6 +116,11 @@ class TaskManager(): for spawn_node in spawn_nodes: kv = spawn_node.get_job_kwargs() job = spawn_node.unified_job_template.create_unified_job(**kv) + # source_workflow_job is a job-specific field rather than a field copied from job + # template, therefore does not fit into the copy routine and should be put outside + # of create_unified_job. + job.source_workflow_job = workflow_job + job.save() spawn_node.job = job spawn_node.save() can_start = job.signal_start(**kv) From 9154f1956054e3642faf135f7ec92979a13bd142 Mon Sep 17 00:00:00 2001 From: Aaron Tan Date: Fri, 25 Nov 2016 21:31:55 -0500 Subject: [PATCH 2/8] Complete related field. --- awx/api/serializers.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 9677c3a62b..a68b413a13 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -598,6 +598,8 @@ class UnifiedJobSerializer(BaseSerializer): res['stdout'] = reverse('api:job_stdout', args=(obj.pk,)) elif isinstance(obj, AdHocCommand): res['stdout'] = reverse('api:ad_hoc_command_stdout', args=(obj.pk,)) + if obj.source_workflow_job: + res['source_workflow_job'] = reverse('api:workflow_job_detail', args=(obj.source_workflow_job.pk,)) return res def to_representation(self, obj): From 3e8e9480d1a5961d56d0ed02ef9295572ef2ae5d Mon Sep 17 00:00:00 2001 From: Aaron Tan Date: Fri, 25 Nov 2016 22:28:52 -0500 Subject: [PATCH 3/8] Pytest fixture adjustment. --- awx/main/tests/unit/api/serializers/test_job_serializers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/awx/main/tests/unit/api/serializers/test_job_serializers.py b/awx/main/tests/unit/api/serializers/test_job_serializers.py index 2ab393f698..d76bf4cfcb 100644 --- a/awx/main/tests/unit/api/serializers/test_job_serializers.py +++ b/awx/main/tests/unit/api/serializers/test_job_serializers.py @@ -34,7 +34,8 @@ def project_update(mocker): @pytest.fixture def job(mocker, job_template, project_update): - return mocker.MagicMock(pk=5, job_template=job_template, project_update=project_update) + return mocker.MagicMock(pk=5, job_template=job_template, project_update=project_update, + source_workflow_job=None) @pytest.fixture From a458bb3c89ad21c9796fe406aa37195f396462dc Mon Sep 17 00:00:00 2001 From: Aaron Tan Date: Tue, 29 Nov 2016 11:51:42 -0500 Subject: [PATCH 4/8] Remove source_workflow_job and use workflow_job_id instead. --- awx/api/generics.py | 5 +++- awx/api/serializers.py | 6 ++--- .../0053_v310_linkage_back_to_workflow_job.py | 20 ---------------- awx/main/models/unified_jobs.py | 8 ------- awx/main/scheduler/__init__.py | 24 +++++++------------ .../api/serializers/test_job_serializers.py | 3 +-- 6 files changed, 17 insertions(+), 49 deletions(-) delete mode 100644 awx/main/migrations/0053_v310_linkage_back_to_workflow_job.py diff --git a/awx/api/generics.py b/awx/api/generics.py index 0c593925c0..8f8d76e3e8 100644 --- a/awx/api/generics.py +++ b/awx/api/generics.py @@ -241,7 +241,10 @@ class ListAPIView(generics.ListAPIView, GenericAPIView): # Base class for a read-only list view. def get_queryset(self): - return self.request.user.get_queryset(self.model) + qs = self.request.user.get_queryset(self.model) + if getattr(self.model, 'spawned_by_workflow', False): + qs = qs.select_related('unified_job_node__workflow_job') + return qs def paginate_queryset(self, queryset): page = super(ListAPIView, self).paginate_queryset(queryset) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index a68b413a13..fde4bdcecb 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -566,7 +566,7 @@ class UnifiedJobSerializer(BaseSerializer): fields = ('*', 'unified_job_template', 'launch_type', 'status', 'failed', 'started', 'finished', 'elapsed', 'job_args', 'job_cwd', 'job_env', 'job_explanation', 'result_stdout', - 'execution_node', 'result_traceback', 'source_workflow_job') + 'execution_node', 'result_traceback', 'workflow_job_id') extra_kwargs = { 'unified_job_template': { 'source': 'unified_job_template_id', @@ -598,8 +598,8 @@ class UnifiedJobSerializer(BaseSerializer): res['stdout'] = reverse('api:job_stdout', args=(obj.pk,)) elif isinstance(obj, AdHocCommand): res['stdout'] = reverse('api:ad_hoc_command_stdout', args=(obj.pk,)) - if obj.source_workflow_job: - res['source_workflow_job'] = reverse('api:workflow_job_detail', args=(obj.source_workflow_job.pk,)) + if obj.workflow_job_id: + res['source_worklflow_job'] = reverse('api:workflow_job_detail', args=(obj.workflow_job_id,)) return res def to_representation(self, obj): diff --git a/awx/main/migrations/0053_v310_linkage_back_to_workflow_job.py b/awx/main/migrations/0053_v310_linkage_back_to_workflow_job.py deleted file mode 100644 index f98840c3e4..0000000000 --- a/awx/main/migrations/0053_v310_linkage_back_to_workflow_job.py +++ /dev/null @@ -1,20 +0,0 @@ -# -*- coding: utf-8 -*- -from __future__ import unicode_literals - -from django.db import migrations, models -import django.db.models.deletion - - -class Migration(migrations.Migration): - - dependencies = [ - ('main', '0052_v310_inventory_name_non_unique'), - ] - - operations = [ - migrations.AddField( - model_name='unifiedjob', - name='source_workflow_job', - field=models.ForeignKey(related_name='spawned_jobs', on_delete=django.db.models.deletion.SET_NULL, default=None, editable=False, to='main.WorkflowJob', null=True), - ), - ] diff --git a/awx/main/models/unified_jobs.py b/awx/main/models/unified_jobs.py index 92903a376e..43cdfd07f0 100644 --- a/awx/main/models/unified_jobs.py +++ b/awx/main/models/unified_jobs.py @@ -528,14 +528,6 @@ class UnifiedJob(PolymorphicModel, PasswordFieldsModel, CommonModelNameNotUnique blank=True, related_name='%(class)s_labels' ) - source_workflow_job = models.ForeignKey( - 'WorkflowJob', - null=True, - default=None, - editable=False, - related_name='spawned_jobs', - on_delete=models.SET_NULL, - ) def get_absolute_url(self): real_instance = self.get_real_instance() diff --git a/awx/main/scheduler/__init__.py b/awx/main/scheduler/__init__.py index ba4aa5eed5..8569fb5cfc 100644 --- a/awx/main/scheduler/__init__.py +++ b/awx/main/scheduler/__init__.py @@ -18,8 +18,8 @@ from awx.main.scheduler.dag_workflow import WorkflowDAG from awx.main.scheduler.dependency_graph import DependencyGraph from awx.main.scheduler.partial import ( - JobDict, - ProjectUpdateDict, + JobDict, + ProjectUpdateDict, ProjectUpdateLatestDict, InventoryUpdateDict, InventoryUpdateLatestDict, @@ -103,7 +103,7 @@ class TaskManager(): for task in all_sorted_tasks: if type(task) is JobDict: inventory_ids.add(task['inventory_id']) - + for inventory_id in inventory_ids: results.append((inventory_id, InventorySourceDict.filter_partial(inventory_id))) @@ -116,11 +116,6 @@ class TaskManager(): for spawn_node in spawn_nodes: kv = spawn_node.get_job_kwargs() job = spawn_node.unified_job_template.create_unified_job(**kv) - # source_workflow_job is a job-specific field rather than a field copied from job - # template, therefore does not fit into the copy routine and should be put outside - # of create_unified_job. - job.source_workflow_job = workflow_job - job.save() spawn_node.job = job spawn_node.save() can_start = job.signal_start(**kv) @@ -179,10 +174,10 @@ class TaskManager(): 'id': task['id'], } dependencies = [{'type': t.get_job_type_str(), 'id': t['id']} for t in dependent_tasks] - + error_handler = handle_work_error.s(subtasks=[task_actual] + dependencies) success_handler = handle_work_success.s(task_actual=task_actual) - + job_obj = task.get_full() job_obj.status = 'waiting' @@ -206,7 +201,7 @@ class TaskManager(): job_obj.websocket_emit_status(job_obj.status) if job_obj.status != 'failed': job_obj.start_celery_task(opts, error_callback=error_handler, success_callback=success_handler) - + connection.on_commit(post_commit) def process_runnable_tasks(self, runnable_tasks): @@ -284,7 +279,7 @@ class TaskManager(): if not self.graph.is_job_blocked(task): dependencies = self.generate_dependencies(task) self.process_dependencies(task, dependencies) - + # Spawning deps might have blocked us if not self.graph.is_job_blocked(task): self.graph.add_job(task) @@ -300,7 +295,7 @@ class TaskManager(): for task in all_running_sorted_tasks: if (task['celery_task_id'] not in active_tasks and not hasattr(settings, 'IGNORE_CELERY_INSPECTOR')): - # NOTE: Pull status again and make sure it didn't finish in + # NOTE: Pull status again and make sure it didn't finish in # the meantime? # TODO: try catch the getting of the job. The job COULD have been deleted task_obj = task.get_full() @@ -351,7 +346,7 @@ class TaskManager(): latest_inventory_updates = self.get_latest_inventory_update_tasks(all_sorted_tasks) self.process_latest_inventory_updates(latest_inventory_updates) - + inventory_id_sources = self.get_inventory_source_tasks(all_sorted_tasks) self.process_inventory_sources(inventory_id_sources) @@ -376,4 +371,3 @@ class TaskManager(): # Operations whose queries rely on modifications made during the atomic scheduling session for wfj in WorkflowJob.objects.filter(id__in=finished_wfjs): _send_notification_templates(wfj, 'succeeded' if wfj.status == 'successful' else 'failed') - diff --git a/awx/main/tests/unit/api/serializers/test_job_serializers.py b/awx/main/tests/unit/api/serializers/test_job_serializers.py index d76bf4cfcb..2ab393f698 100644 --- a/awx/main/tests/unit/api/serializers/test_job_serializers.py +++ b/awx/main/tests/unit/api/serializers/test_job_serializers.py @@ -34,8 +34,7 @@ def project_update(mocker): @pytest.fixture def job(mocker, job_template, project_update): - return mocker.MagicMock(pk=5, job_template=job_template, project_update=project_update, - source_workflow_job=None) + return mocker.MagicMock(pk=5, job_template=job_template, project_update=project_update) @pytest.fixture From 88bf9753688b9aec40a6f2aa8c63f74fb7d647f1 Mon Sep 17 00:00:00 2001 From: Aaron Tan Date: Tue, 29 Nov 2016 12:58:04 -0500 Subject: [PATCH 5/8] Pytest fixture adjustment. --- awx/main/tests/unit/api/serializers/test_job_serializers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/awx/main/tests/unit/api/serializers/test_job_serializers.py b/awx/main/tests/unit/api/serializers/test_job_serializers.py index 2ab393f698..603b72892c 100644 --- a/awx/main/tests/unit/api/serializers/test_job_serializers.py +++ b/awx/main/tests/unit/api/serializers/test_job_serializers.py @@ -34,7 +34,8 @@ def project_update(mocker): @pytest.fixture def job(mocker, job_template, project_update): - return mocker.MagicMock(pk=5, job_template=job_template, project_update=project_update) + return mocker.MagicMock(pk=5, job_template=job_template, project_update=project_update, + workflow_job_id=None) @pytest.fixture From 908b140d2857a80aae2434e64a166ca4cbef3cac Mon Sep 17 00:00:00 2001 From: Aaron Tan Date: Thu, 1 Dec 2016 10:50:18 -0500 Subject: [PATCH 6/8] Add back linkage to summary fields and remove it from normal fields. --- awx/api/serializers.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index fde4bdcecb..374efb869a 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -566,7 +566,7 @@ class UnifiedJobSerializer(BaseSerializer): fields = ('*', 'unified_job_template', 'launch_type', 'status', 'failed', 'started', 'finished', 'elapsed', 'job_args', 'job_cwd', 'job_env', 'job_explanation', 'result_stdout', - 'execution_node', 'result_traceback', 'workflow_job_id') + 'execution_node', 'result_traceback') extra_kwargs = { 'unified_job_template': { 'source': 'unified_job_template_id', @@ -602,6 +602,18 @@ class UnifiedJobSerializer(BaseSerializer): res['source_worklflow_job'] = reverse('api:workflow_job_detail', args=(obj.workflow_job_id,)) return res + def get_summary_fields(self, obj): + summary_fields = super(UnifiedJobSerializer, self).get_summary_fields(obj) + if obj.spawned_by_workflow: + summary_fields['source_worklflow_job'] = {} + summary_obj = obj.unified_job_node.workflow_job + for field in SUMMARIZABLE_FK_FIELDS['job']: + val = getattr(summary_obj, field, None) + if val is not None: + summary_fields['source_worklflow_job'][field] = val + + return summary_fields + def to_representation(self, obj): serializer_class = None if type(self) is UnifiedJobSerializer: From e63716c0bbe166c600c1ea939dfa946b79baf802 Mon Sep 17 00:00:00 2001 From: Aaron Tan Date: Thu, 1 Dec 2016 11:04:06 -0500 Subject: [PATCH 7/8] Move performance boost to access.py. --- awx/api/generics.py | 5 +---- awx/api/serializers.py | 6 +++--- awx/main/access.py | 2 ++ 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/awx/api/generics.py b/awx/api/generics.py index 8f8d76e3e8..0c593925c0 100644 --- a/awx/api/generics.py +++ b/awx/api/generics.py @@ -241,10 +241,7 @@ class ListAPIView(generics.ListAPIView, GenericAPIView): # Base class for a read-only list view. def get_queryset(self): - qs = self.request.user.get_queryset(self.model) - if getattr(self.model, 'spawned_by_workflow', False): - qs = qs.select_related('unified_job_node__workflow_job') - return qs + return self.request.user.get_queryset(self.model) def paginate_queryset(self, queryset): page = super(ListAPIView, self).paginate_queryset(queryset) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 374efb869a..1cea607111 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -599,18 +599,18 @@ class UnifiedJobSerializer(BaseSerializer): elif isinstance(obj, AdHocCommand): res['stdout'] = reverse('api:ad_hoc_command_stdout', args=(obj.pk,)) if obj.workflow_job_id: - res['source_worklflow_job'] = reverse('api:workflow_job_detail', args=(obj.workflow_job_id,)) + res['source_workflow_job'] = reverse('api:workflow_job_detail', args=(obj.workflow_job_id,)) return res def get_summary_fields(self, obj): summary_fields = super(UnifiedJobSerializer, self).get_summary_fields(obj) if obj.spawned_by_workflow: - summary_fields['source_worklflow_job'] = {} + summary_fields['source_workflow_job'] = {} summary_obj = obj.unified_job_node.workflow_job for field in SUMMARIZABLE_FK_FIELDS['job']: val = getattr(summary_obj, field, None) if val is not None: - summary_fields['source_worklflow_job'][field] = val + summary_fields['source_workflow_job'][field] = val return summary_fields diff --git a/awx/main/access.py b/awx/main/access.py index 9059b4c7d0..eb50fb244d 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -1847,6 +1847,8 @@ class UnifiedJobAccess(BaseAccess): qs = qs.prefetch_related( 'unified_job_template', ) + if self.model.spawned_by_workflow: + qs = qs.select_related('unified_job_node__workflow_job') # WISH - sure would be nice if the following worked, but it does not. # In the future, as django and polymorphic libs are upgraded, try again. From 9f629048dad55c98d2d0b5a19605d1cbea6ee3ac Mon Sep 17 00:00:00 2001 From: Aaron Tan Date: Thu, 1 Dec 2016 15:05:22 -0500 Subject: [PATCH 8/8] Simplify select_related. --- awx/main/access.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index eb50fb244d..4ffbcf288b 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -1843,12 +1843,11 @@ class UnifiedJobAccess(BaseAccess): qs = qs.select_related( 'created_by', 'modified_by', + 'unified_job_node__workflow_job', ) qs = qs.prefetch_related( 'unified_job_template', ) - if self.model.spawned_by_workflow: - qs = qs.select_related('unified_job_node__workflow_job') # WISH - sure would be nice if the following worked, but it does not. # In the future, as django and polymorphic libs are upgraded, try again.