From 0c81ed8e76671019e4e09a1e96cd5cb6cce753c3 Mon Sep 17 00:00:00 2001 From: William Lachance Date: Wed, 20 Dec 2017 19:37:07 -0500 Subject: [PATCH] Bug 1328985 - Remove project_specific_id compatibility shim (#3030) --- tests/etl/test_perf_data_adapters.py | 6 +- tests/test_utils.py | 4 +- tests/webapp/api/test_job_details_api.py | 2 +- tests/webapp/api/test_job_log_url_api.py | 4 +- tests/webapp/api/test_jobs_api.py | 20 ----- .../0020_remove_project_specific_id.py | 28 ++++++ treeherder/model/models.py | 6 +- treeherder/webapp/api/jobs.py | 3 - ui/js/controllers/logviewer.js | 89 +++++++++---------- 9 files changed, 78 insertions(+), 84 deletions(-) create mode 100644 treeherder/model/migrations/0020_remove_project_specific_id.py diff --git a/tests/etl/test_perf_data_adapters.py b/tests/etl/test_perf_data_adapters.py index 608065bb8..ceecb7716 100644 --- a/tests/etl/test_perf_data_adapters.py +++ b/tests/etl/test_perf_data_adapters.py @@ -28,7 +28,7 @@ def perf_push(test_repository): @pytest.fixture def perf_job(perf_push, failure_classifications, generic_reference_data): return create_generic_job('myfunguid', perf_push.repository, - perf_push.id, 1, generic_reference_data) + perf_push.id, generic_reference_data) def _generate_perf_data_range(test_repository, @@ -52,7 +52,7 @@ def _generate_perf_data_range(test_repository, author='foo@bar.com', time=push_time) job = create_generic_job('myguid%s' % i, test_repository, - push.id, i, generic_reference_data) + push.id, generic_reference_data) datum = { 'job_guid': 'fake_job_guid', 'name': 'test', @@ -244,7 +244,7 @@ def test_load_generic_data(test_repository, author='foo@bar.com', time=later_timestamp) later_job = create_generic_job('lateguid', test_repository, - later_push.id, 2, generic_reference_data) + later_push.id, generic_reference_data) store_performance_artifact(later_job, submit_datum) signature = PerformanceSignature.objects.get( suite=perf_datum['suites'][0]['name'], diff --git a/tests/test_utils.py b/tests/test_utils.py index f1e0a59de..d48d5c4ab 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -220,8 +220,7 @@ def load_exp(filename): return {} -def create_generic_job(guid, repository, push_id, project_specific_id, - generic_reference_data): +def create_generic_job(guid, repository, push_id, generic_reference_data): job_time = datetime.datetime.fromtimestamp(0) return models.Job.objects.create( guid=guid, @@ -243,7 +242,6 @@ def create_generic_job(guid, repository, push_id, project_specific_id, submit_time=job_time, start_time=job_time, end_time=job_time, - project_specific_id=project_specific_id, tier=1) diff --git a/tests/webapp/api/test_job_details_api.py b/tests/webapp/api/test_job_details_api.py index c72c220dd..bcddafb9e 100644 --- a/tests/webapp/api/test_job_details_api.py +++ b/tests/webapp/api/test_job_details_api.py @@ -45,7 +45,7 @@ def test_job_details(test_repository, failure_classifications, push_id = 2 i = 1 print (i, repository) - job = create_generic_job(job_guid, repository, push_id, i, + job = create_generic_job(job_guid, repository, push_id, generic_reference_data) JobDetail.objects.create( job=job, **params) diff --git a/tests/webapp/api/test_job_log_url_api.py b/tests/webapp/api/test_job_log_url_api.py index 360a30599..624c012ee 100644 --- a/tests/webapp/api/test_job_log_url_api.py +++ b/tests/webapp/api/test_job_log_url_api.py @@ -7,9 +7,9 @@ from treeherder.model.models import JobLog def test_get_job_log_urls(test_repository, push_stored, failure_classifications, generic_reference_data, webapp): - job1 = create_generic_job('1234', test_repository, 1, 1, + job1 = create_generic_job('1234', test_repository, 1, generic_reference_data) - job2 = create_generic_job('5678', test_repository, 1, 2, + job2 = create_generic_job('5678', test_repository, 1, generic_reference_data) JobLog.objects.create(job=job1, diff --git a/tests/webapp/api/test_jobs_api.py b/tests/webapp/api/test_jobs_api.py index 8ebcb999c..370153427 100644 --- a/tests/webapp/api/test_jobs_api.py +++ b/tests/webapp/api/test_jobs_api.py @@ -158,26 +158,6 @@ def test_job_list_filter_fields(webapp, eleven_jobs_stored, test_repository, fie assert first[fieldname] == expected -def test_job_list_filter_project_specific_id(webapp, test_job): - """ - tests retrieving by project specific id - - This is used by the logviewer in the front end so that old generated - urls still work - """ - # resave the test_job with a different project specific id - test_job.project_specific_id = test_job.id + 1 - test_job.save() - - url = reverse("jobs-list", - kwargs={"project": test_job.repository.name}) - url += "?project_specific_id={}".format(test_job.project_specific_id) - resp = webapp.get(url).json - - assert len(resp['results']) == 1 - assert resp['results'][0]['id'] == test_job.id - - def test_job_list_in_filter(webapp, eleven_jobs_stored, test_repository): """ test retrieving a job list with a querystring filter. diff --git a/treeherder/model/migrations/0020_remove_project_specific_id.py b/treeherder/model/migrations/0020_remove_project_specific_id.py new file mode 100644 index 000000000..06424a2fd --- /dev/null +++ b/treeherder/model/migrations/0020_remove_project_specific_id.py @@ -0,0 +1,28 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.6 on 2017-12-08 22:20 +from __future__ import unicode_literals + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('model', '0019_remove_job_duration'), + ] + operations = [ + migrations.RunSQL( + sql=""" +ALTER TABLE `job` DROP INDEX `job_repository_id_project_specific_id_7883a5e8_uniq`; + """, + reverse_sql=""" +CREATE UNIQUE INDEX `job_repository_id_project_specific_id_7883a5e8_uni` ON `job` (`repository_id`,`project_specific_id`); + """, + state_operations=[ + migrations.AlterUniqueTogether( + name='job', + unique_together=set([]), + ), + ] + ) + ] diff --git a/treeherder/model/models.py b/treeherder/model/models.py index a33ab864d..3ee2b41de 100644 --- a/treeherder/model/models.py +++ b/treeherder/model/models.py @@ -448,7 +448,7 @@ class Job(models.Model): repository = models.ForeignKey(Repository, on_delete=models.CASCADE) guid = models.CharField(max_length=50, unique=True) - project_specific_id = models.PositiveIntegerField(null=True) + project_specific_id = models.PositiveIntegerField(null=True) # unused, see bug 1328985 autoclassify_status = models.IntegerField(choices=AUTOCLASSIFY_STATUSES, default=PENDING) coalesced_to_guid = models.CharField(max_length=50, null=True, @@ -479,7 +479,6 @@ class Job(models.Model): class Meta: db_table = 'job' - unique_together = ('repository', 'project_specific_id') index_together = [ # these speed up the various permutations of the "similar jobs" # queries @@ -496,8 +495,7 @@ class Job(models.Model): ] def __str__(self): - return "{0} {1} {2} {3}".format(self.id, self.repository, self.guid, - self.project_specific_id) + return "{0} {1} {2}".format(self.id, self.repository, self.guid) def get_platform_option(self, option_collection_map=None): if not hasattr(self, 'platform_option'): diff --git a/treeherder/webapp/api/jobs.py b/treeherder/webapp/api/jobs.py index 850e9b544..8aa5bd6ac 100644 --- a/treeherder/webapp/api/jobs.py +++ b/treeherder/webapp/api/jobs.py @@ -39,8 +39,6 @@ class JobFilter(django_filters.FilterSet): as the previous jobs API """ id = django_filters.NumberFilter(name='id') - project_specific_id = django_filters.NumberFilter( - name='project_specific_id') id__in = NumberInFilter(name='id', lookup_expr='in') tier__in = NumberInFilter(name='tier', lookup_expr='in') push_id__in = NumberInFilter(name='push_id', lookup_expr='in') @@ -86,7 +84,6 @@ class JobFilter(django_filters.FilterSet): 'failure_classification_id': ['exact'], 'job_type_id': ['exact'], 'job_group_id': ['exact'], - 'project_specific_id': ['exact'], 'reason': ['exact'], 'state': ['exact'], 'result': ['exact'], diff --git a/ui/js/controllers/logviewer.js b/ui/js/controllers/logviewer.js index 311155df2..69a6563f3 100644 --- a/ui/js/controllers/logviewer.js +++ b/ui/js/controllers/logviewer.js @@ -107,60 +107,53 @@ logViewerApp.controller('LogviewerCtrl', [ $scope.init = () => { $scope.logProperties = []; - ThJobModel.get_list($scope.repoName, { - project_specific_id: $scope.job_id - }).then(function (jobList) { - if (jobList.length > 0) { - $scope.job_id = jobList[0].id; + ThJobModel.get($scope.repoName, $scope.job_id).then((job) => { + // set the title of the browser window/tab + $scope.logViewerTitle = job.get_title(); + + if (job.logs && job.logs.length) { + $scope.rawLogURL = job.logs[0].url; } - ThJobModel.get($scope.repoName, $scope.job_id).then((job) => { - // set the title of the browser window/tab - $scope.logViewerTitle = job.get_title(); - if (job.logs && job.logs.length) { - $scope.rawLogURL = job.logs[0].url; + // set the result value and shading color class + $scope.result = { label: 'Result', value: job.result }; + $scope.resultStatusShading = $scope.getShadingClass(job.result); + + // other properties, in order of appearance + $scope.logProperties = [ + { label: 'Job', value: $scope.logViewerTitle }, + { label: 'Machine', value: job.machine_name }, + { label: 'Start', value: dateFilter(job.start_timestamp * 1000, thDateFormat) }, + { label: 'End', value: dateFilter(job.end_timestamp * 1000, thDateFormat) } + ]; + + // Test to disable successful steps checkbox on taskcluster jobs + $scope.isTaskClusterLog = (job.build_system_type === 'taskcluster'); + if (job.taskcluster_metadata) { + $scope.taskId = job.taskcluster_metadata.task_id; + } + + // Test to expose the reftest button in the logviewer actionbar + $scope.isReftest = () => { + if (job.job_group_name) { + return thReftestStatus(job); } + }; - // set the result value and shading color class - $scope.result = { label: 'Result', value: job.result }; - $scope.resultStatusShading = $scope.getShadingClass(job.result); + // get the revision and linkify it + ThResultSetModel.getResultSet($scope.repoName, job.result_set_id).then((data) => { + const revision = data.data.revision; - // other properties, in order of appearance - $scope.logProperties = [ - { label: 'Job', value: $scope.logViewerTitle }, - { label: 'Machine', value: job.machine_name }, - { label: 'Start', value: dateFilter(job.start_timestamp * 1000, thDateFormat) }, - { label: 'End', value: dateFilter(job.end_timestamp * 1000, thDateFormat) } - ]; - - // Test to disable successful steps checkbox on taskcluster jobs - $scope.isTaskClusterLog = (job.build_system_type === 'taskcluster'); - if (job.taskcluster_metadata) { - $scope.taskId = job.taskcluster_metadata.task_id; - } - - // Test to expose the reftest button in the logviewer actionbar - $scope.isReftest = () => { - if (job.job_group_name) { - return thReftestStatus(job); - } - }; - - // get the revision and linkify it - ThResultSetModel.getResultSet($scope.repoName, job.result_set_id).then((data) => { - const revision = data.data.revision; - - $scope.logProperties.push({ label: 'Revision', value: revision }); - }); - - ThJobDetailModel.getJobDetails({ job_guid: job.job_guid }).then((jobDetails) => { - $scope.job_details = jobDetails; - }); - }, () => { - $scope.loading = false; - $scope.jobExists = false; - thNotify.send('The job does not exist or has expired', 'danger', { sticky: true }); + $scope.logProperties.push({ label: 'Revision', value: revision }); }); + + ThJobDetailModel.getJobDetails({ job_guid: job.job_guid }).then((jobDetails) => { + $scope.job_details = jobDetails; + }); + }, () => { + $scope.loading = false; + $scope.jobExists = false; + thNotify.send('The job does not exist or has expired', 'danger', { sticky: true }); }); };