From a468baf560c32fe764726d1a72895f562c430887 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Thu, 4 Apr 2019 10:45:31 -0700 Subject: [PATCH] Fix SQL error when querying for tuples When requiring more than one artifact zuul runs into an sql exception [1] which bubbles up to the run_handler. This effectively blocks all operations of zuul until the change that triggers this bug is dequeued. Fix this by correctly filtering the sql results. [1] Trace 2019-04-04 17:15:01,158 ERROR zuul.Scheduler: Exception in run handler: Traceback (most recent call last): File "/opt/zuul/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1244, in _execute_context cursor, statement, parameters, context File "/opt/zuul/lib/python3.7/site-packages/sqlalchemy/engine/default.py", line 552, in do_execute cursor.execute(statement, parameters) psycopg2.ProgrammingError: operator does not exist: character varying = record LINE 3: ...bc89ecf79fd84fc7c' AND zuul_provides.name = ('pro... ^ HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts. Change-Id: I7f95e1376b7a1f46a5b4ef5242c777e16ceca451 Co-Authored-By: Tobias Henkel --- tests/fixtures/layouts/provides-requires.yaml | 13 ++++ tests/unit/test_v3.py | 66 +++++++++++++++++++ zuul/driver/sql/sqlconnection.py | 2 +- 3 files changed, 80 insertions(+), 1 deletion(-) diff --git a/tests/fixtures/layouts/provides-requires.yaml b/tests/fixtures/layouts/provides-requires.yaml index b60dd16e2d..655466eb60 100644 --- a/tests/fixtures/layouts/provides-requires.yaml +++ b/tests/fixtures/layouts/provides-requires.yaml @@ -57,6 +57,12 @@ name: library-user requires: libraries +- job: + name: both-user + requires: + - images + - libraries + - job: name: hold @@ -83,3 +89,10 @@ queue: integrated jobs: - image-user + +- project: + name: org/project3 + check: + jobs: + - both-user + - hold diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index 17f73682d7..d9700ba31b 100644 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -5521,6 +5521,28 @@ class TestProvidesRequires(ZuulDBTestCase): changes='1,1 2,1 3,1'), dict(name='hold', result='SUCCESS', changes='1,1 2,1 3,1'), ], ordered=False) + + D = self.fake_gerrit.addFakeChange('org/project3', 'master', 'D') + D.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( + D.subject, B.data['id']) + self.fake_gerrit.addEvent(D.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + self.assertHistory([ + dict(name='image-builder', result='SUCCESS', changes='1,1'), + dict(name='library-builder', result='SUCCESS', changes='1,1'), + dict(name='hold', result='SUCCESS', changes='1,1'), + dict(name='image-builder', result='SUCCESS', changes='1,1 2,1'), + dict(name='library-builder', result='SUCCESS', changes='1,1 2,1'), + dict(name='hold', result='SUCCESS', changes='1,1 2,1'), + dict(name='image-user', result='SUCCESS', changes='1,1 2,1 3,1'), + dict(name='library-user', result='SUCCESS', + changes='1,1 2,1 3,1'), + dict(name='hold', result='SUCCESS', changes='1,1 2,1 3,1'), + dict(name='both-user', result='SUCCESS', changes='1,1 2,1 4,1'), + dict(name='hold', result='SUCCESS', changes='1,1 2,1 4,1'), + ], ordered=False) + image_user = self.getJobFromHistory('image-user') self.assertEqual( image_user.parameters['zuul']['artifacts'], @@ -5569,6 +5591,50 @@ class TestProvidesRequires(ZuulDBTestCase): 'type': 'library_object', } }]) + both_user = self.getJobFromHistory('both-user') + self.assertEqual( + both_user.parameters['zuul']['artifacts'], + [{ + 'project': 'org/project1', + 'change': '1', + 'patchset': '1', + 'job': 'image-builder', + 'url': 'http://example.com/image', + 'name': 'image', + 'metadata': { + 'type': 'container_image', + } + }, { + 'project': 'org/project1', + 'change': '1', + 'patchset': '1', + 'job': 'library-builder', + 'url': 'http://example.com/library', + 'name': 'library', + 'metadata': { + 'type': 'library_object', + } + }, { + 'project': 'org/project1', + 'change': '2', + 'patchset': '1', + 'job': 'image-builder', + 'url': 'http://example.com/image2', + 'name': 'image2', + 'metadata': { + 'type': 'container_image', + } + }, { + 'project': 'org/project1', + 'change': '2', + 'patchset': '1', + 'job': 'library-builder', + 'url': 'http://example.com/library2', + 'name': 'library2', + 'metadata': { + 'type': 'library_object', + } + }]) @simple_layout('layouts/provides-requires.yaml') def test_provides_requires_check_old_failure(self): diff --git a/zuul/driver/sql/sqlconnection.py b/zuul/driver/sql/sqlconnection.py index 96a87620d0..1259d713bb 100644 --- a/zuul/driver/sql/sqlconnection.py +++ b/zuul/driver/sql/sqlconnection.py @@ -52,7 +52,7 @@ class DatabaseSession(object): def listFilter(self, query, column, value): if value is None: return query - if isinstance(value, list): + if isinstance(value, list) or isinstance(value, tuple): return query.filter(column.in_(value)) return query.filter(column == value)