Merge lp:~ursinha/uci-engine/update-ticket-raises-error into lp:uci-engine

Proposed by Ursula Junque
Status: Merged
Approved by: Ursula Junque
Approved revision: 648
Merged at revision: 646
Proposed branch: lp:~ursinha/uci-engine/update-ticket-raises-error
Merge into: lp:uci-engine
Diff against target: 619 lines (+208/-74)
9 files modified
cli/ci_cli/tests/__init__.py (+2/-2)
cli/ci_cli/tests/test_cli.py (+81/-19)
cli/ci_cli/tests/test_get_ticket_status.py (+6/-6)
cli/ci_cli/tests/test_image.py (+11/-11)
cli/ci_cli/tests/test_ticket.py (+18/-18)
cli/ci_cli/tests/test_utils.py (+61/-3)
cli/ci_cli/ticket.py (+18/-7)
cli/ci_cli/utils.py (+9/-7)
cli/ubuntu-ci (+2/-1)
To merge this branch: bzr merge lp:~ursinha/uci-engine/update-ticket-raises-error
Reviewer Review Type Date Requested Status
Francis Ginther Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+225593@code.launchpad.net

Commit message

Display better errors in case requests fail during ticket creation

Description of the change

This branch ensures all requests will raise exceptions if their return codes are errors. What happened in bug 1337294 should be a rare problem, and until we understand what happened it's hard to find a definitive solution. If that happens again, now it's explicit that the problem happens while update_ticket, and while we don't have a retry mechanism I'm directing the user to let an admin know what happened, to be able to manually set it to "Queued".

To post a comment you must log in.
647. By Ursula Junque

Oops, test case was named wrong

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:647
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1013/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1013/rebuild

review: Approve (continuous-integration)
Revision history for this message
Andy Doan (doanac) wrote :

I have a question about what we print out during a failure. Since I'll be off tomorrow, don't let my question block things if you have sorted out.

Revision history for this message
Ursula Junque (ursinha) wrote :

I've replied inline, thanks Andy.

648. By Ursula Junque

Fixing variable names on cli utils, there were misleading

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:648
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1015/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1015/rebuild

review: Approve (continuous-integration)
Revision history for this message
Francis Ginther (fginther) wrote :

This is a step forward and I'll approve. I would like to make sure we take a closer look at the other post and get calls within the cli (and indeed all code) and make sure we're not dropping relevant error messages.

review: Approve
Revision history for this message
Ursula Junque (ursinha) wrote :

> This is a step forward and I'll approve. I would like to make sure we take a
> closer look at the other post and get calls within the cli (and indeed all
> code) and make sure we're not dropping relevant error messages.

All of the posts, gets and patches are covered now, using utils.patch, utils.post and utils.get will always raise_for_status and main cli script will catch and print that. I agree we still need to revisit how we handle exceptions to see if we're not losing important information we could be displaying.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cli/ci_cli/tests/__init__.py'
2--- cli/ci_cli/tests/__init__.py 2014-06-19 23:38:34 +0000
3+++ cli/ci_cli/tests/__init__.py 2014-07-04 03:45:58 +0000
4@@ -45,8 +45,8 @@
5 sys.stdout = out
6
7
8-class FakeRequestsReturn:
9- """Object to mock the return of python-requests return."""
10+class FakeResponse:
11+ """Object to mock a python-requests response."""
12
13 def __init__(self, code, content, reason=None):
14 self.status_code = code
15
16=== modified file 'cli/ci_cli/tests/test_cli.py'
17--- cli/ci_cli/tests/test_cli.py 2014-07-01 17:02:35 +0000
18+++ cli/ci_cli/tests/test_cli.py 2014-07-04 03:45:58 +0000
19@@ -29,7 +29,7 @@
20 capture_stderr,
21 capture_stdout,
22 MainScriptTestCase,
23- FakeRequestsReturn,
24+ FakeResponse,
25 )
26 from ci_utils.testing import get_test_file_path
27 from ci_utils.ticket_states import (
28@@ -62,7 +62,7 @@
29 "removed_binaries": "myoldpackage",
30 }
31 mock_requests.side_effect = [
32- FakeRequestsReturn(200, json.dumps(data))
33+ FakeResponse(200, json.dumps(data))
34 ]
35 args = ['status', '-t', '5']
36 with capture_stdout(self.cli.main, args) as cm:
37@@ -101,7 +101,7 @@
38 ]
39 }
40 mock_requests.side_effect = [
41- FakeRequestsReturn(200, json.dumps(data))
42+ FakeResponse(200, json.dumps(data))
43 ]
44 args = ['status']
45 with capture_stdout(self.cli.main, args) as cm:
46@@ -159,7 +159,7 @@
47 def test_ts_server_returns_urlerror(self, mock_requests):
48 """Test cli response to ts server refusing connection (111)."""
49 mock_requests.side_effect = [
50- FakeRequestsReturn(
51+ FakeResponse(
52 400, '', socket.error(111, "Connection refused"))
53 ]
54 args = ['status']
55@@ -176,7 +176,7 @@
56 def test_ts_server_returns_internal_server_error(self, mock_requests):
57 """Test cli response to ts server returning error 500."""
58 mock_requests.side_effect = [
59- FakeRequestsReturn(
60+ FakeResponse(
61 500, "", "INTERNAL SERVER ERROR"
62 )
63 ]
64@@ -193,7 +193,7 @@
65 def test_ts_server_returns_url_not_found(self, mock_requests):
66 """Test cli response to ts server returning 404 error."""
67 mock_requests.side_effect = [
68- FakeRequestsReturn(404, '', 'Not Found')
69+ FakeResponse(404, '', 'Not Found')
70 ]
71 args = ['status']
72 with LogCapture() as lc:
73@@ -221,7 +221,7 @@
74 def test_ts_server_returns_other_httperror(self, mock_requests):
75 """Test cli response to ts server returning HTTPError."""
76 mock_requests.side_effect = [
77- FakeRequestsReturn(403, '', 'Forbidden')
78+ FakeResponse(403, '', 'Forbidden')
79 ]
80 args = ['status']
81 with LogCapture() as lc:
82@@ -246,18 +246,18 @@
83 # CLI 'create_ticket' action allows users to upload new sources
84 # via gatekeeper (without swift credentials).
85 mocked_post.side_effect = [
86- FakeRequestsReturn(201, '/ticket/1/sandbox/UUID'),
87- FakeRequestsReturn(201, '/ticket.html?ticket_id=1'),
88+ FakeResponse(201, '/ticket/1/sandbox/UUID'),
89+ FakeResponse(201, '/ticket.html?ticket_id=1'),
90 ]
91 mocked_put.side_effect = [
92- FakeRequestsReturn(200, 'A_TEMP_URL'),
93- FakeRequestsReturn(201, '/a_sandbox_object_url/'),
94- FakeRequestsReturn(200, 'A_TEMP_URL'),
95- FakeRequestsReturn(201, '/a_sandbox_object_url/'),
96- FakeRequestsReturn(200, 'A_TEMP_URL'),
97- FakeRequestsReturn(201, '/a_sandbox_object_url/'),
98- FakeRequestsReturn(200, 'A_TEMP_URL'),
99- FakeRequestsReturn(201, '/a_sandbox_object_url/'),
100+ FakeResponse(200, 'A_TEMP_URL'),
101+ FakeResponse(201, '/a_sandbox_object_url/'),
102+ FakeResponse(200, 'A_TEMP_URL'),
103+ FakeResponse(201, '/a_sandbox_object_url/'),
104+ FakeResponse(200, 'A_TEMP_URL'),
105+ FakeResponse(201, '/a_sandbox_object_url/'),
106+ FakeResponse(200, 'A_TEMP_URL'),
107+ FakeResponse(201, '/a_sandbox_object_url/'),
108 ]
109 mocked_of.return_value = 'a_content'
110 foobar_changes = get_test_file_path('foobar_0.1-1_source.changes')
111@@ -272,7 +272,8 @@
112 # The first output line contains a local absolute host path, so
113 # it is ignored.
114 self.assertEquals(
115- ['Validating .changes file...',
116+ ['Parsing foobar_0.1-1_source.changes...',
117+ 'Validating .changes...',
118 'foobar_0.1-1 parsed and validated.',
119 'Created source package upload: http://mocked/api/v1/ticket/1/',
120 'Created subticket: http://mocked/api/v1/ticket/1/',
121@@ -285,7 +286,68 @@
122 'Created artifact: http://mocked/api/v1/ticket/1/',
123 'Created artifact: http://mocked/api/v1/ticket/1/',
124 'Created artifact: http://mocked/api/v1/ticket/1/'],
125- list([r.msg for r in lc.records])[1:])
126+ list([r.msg for r in lc.records]))
127+
128+ @mock.patch('ci_cli.utils.post',
129+ return_value='http://mocked/api/v1/ticket/1/')
130+ @mock.patch('ci_cli.utils.get')
131+ @mock.patch('ci_cli.ticket._open_file')
132+ @mock.patch('requests.put')
133+ @mock.patch('requests.post')
134+ @mock.patch('requests.patch')
135+ def test_create_ticket_update_fails(
136+ self, mocked_patch, mocked_post, mocked_put, mocked_of, mocked_get,
137+ mocked_upost):
138+ # CLI 'create_ticket' action allows users to upload new sources
139+ # via gatekeeper (without swift credentials).
140+ mocked_post.side_effect = [
141+ FakeResponse(201, '/ticket/1/sandbox/UUID'),
142+ FakeResponse(201, '/ticket.html?ticket_id=1'),
143+ ]
144+ mocked_put.side_effect = [
145+ FakeResponse(200, 'A_TEMP_URL'),
146+ FakeResponse(201, '/a_sandbox_object_url/'),
147+ FakeResponse(200, 'A_TEMP_URL'),
148+ FakeResponse(201, '/a_sandbox_object_url/'),
149+ FakeResponse(200, 'A_TEMP_URL'),
150+ FakeResponse(201, '/a_sandbox_object_url/'),
151+ FakeResponse(200, 'A_TEMP_URL'),
152+ FakeResponse(201, '/a_sandbox_object_url/'),
153+ ]
154+ mocked_of.return_value = 'a_content'
155+ foobar_changes = get_test_file_path('foobar_0.1-1_source.changes')
156+ args = ['-v2', 'create_ticket', '-t', '"New feature"', '-d',
157+ '"New feature description"', '-o', 'someone@example.com',
158+ '-a', 'foobar', '-s', foobar_changes]
159+ mocked_patch.side_effect = [
160+ FakeResponse(401, {}, "Failed to patch ticket")]
161+ with LogCapture() as lc:
162+ logger = logging.getLogger()
163+ self.cli.main(args, log=logger)
164+ ticket_url = urlparse.urljoin(
165+ utils.CI_URL, '/ticket.html?ticket_id=1')
166+ # The first output line contains a local absolute host path, so
167+ # it is ignored.
168+ self.assertEquals(
169+ ['Parsing foobar_0.1-1_source.changes...',
170+ 'Validating .changes...',
171+ 'foobar_0.1-1 parsed and validated.',
172+ 'Created source package upload: http://mocked/api/v1/ticket/1/',
173+ 'Created subticket: http://mocked/api/v1/ticket/1/',
174+ 'Uploading foobar_0.1-1.dsc ...',
175+ 'Uploading foobar_0.1.orig.tar.gz ...',
176+ 'Uploading foobar_0.1-1.debian.tar.gz ...',
177+ 'Uploading foobar_0.1-1_source.changes ...',
178+ 'Done! Access {} for more information.'.format(ticket_url),
179+ 'Created artifact: http://mocked/api/v1/ticket/1/',
180+ 'Created artifact: http://mocked/api/v1/ticket/1/',
181+ 'Created artifact: http://mocked/api/v1/ticket/1/',
182+ 'Created artifact: http://mocked/api/v1/ticket/1/',
183+ 'Ticket was created successfully but there was a problem adding '
184+ 'it to the queue: 401 Client Error: Failed to patch ticket. This '
185+ 'should be a transient error, please contact an admin. Your '
186+ 'ticket number is 1.'],
187+ list([r.msg for r in lc.records]))
188
189
190 class CliArgumentsTestCase(MainScriptTestCase):
191
192=== modified file 'cli/ci_cli/tests/test_get_ticket_status.py'
193--- cli/ci_cli/tests/test_get_ticket_status.py 2014-05-26 13:47:38 +0000
194+++ cli/ci_cli/tests/test_get_ticket_status.py 2014-07-04 03:45:58 +0000
195@@ -21,7 +21,7 @@
196 from ci_cli.tests import (
197 capture_stdout,
198 MainScriptTestCase,
199- FakeRequestsReturn,
200+ FakeResponse,
201 )
202 from ci_utils.ticket_states import (
203 TicketWorkflowStep,
204@@ -47,7 +47,7 @@
205 "removed_binaries": "myoldpackage",
206 }
207 mock_requests.side_effect = [
208- FakeRequestsReturn(200, json.dumps(data))
209+ FakeResponse(200, json.dumps(data))
210 ]
211 args = self.cli.parse_arguments(['status', '-t', '100'])
212 with capture_stdout(args.func, args) as cm:
213@@ -82,7 +82,7 @@
214 "removed_binaries": "myoldpackage",
215 }
216 mock_requests.side_effect = [
217- FakeRequestsReturn(200, json.dumps(data))
218+ FakeResponse(200, json.dumps(data))
219 ]
220 args = self.cli.parse_arguments(['status', '-t', '100'])
221 with capture_stdout(args.func, args) as cm:
222@@ -117,7 +117,7 @@
223 "removed_binaries": "myoldpackage",
224 }
225 mock_requests.side_effect = [
226- FakeRequestsReturn(200, json.dumps(data))
227+ FakeResponse(200, json.dumps(data))
228 ]
229 args = self.cli.parse_arguments(['status', '-t', '100'])
230 with capture_stdout(args.func, args) as cm:
231@@ -166,7 +166,7 @@
232 ]
233 }
234 mock_requests.side_effect = [
235- FakeRequestsReturn(200, json.dumps(data))
236+ FakeResponse(200, json.dumps(data))
237 ]
238 args = self.cli.parse_arguments(['status'])
239 with capture_stdout(args.func, args) as cm:
240@@ -181,7 +181,7 @@
241 @mock.patch('requests.get')
242 def test_get_status_404_response(self, mock_requests):
243 mock_requests.side_effect = [
244- FakeRequestsReturn(404, '', 'Not Found')
245+ FakeResponse(404, '', 'Not Found')
246 ]
247 args = self.cli.parse_arguments(['status', '-t', '99'])
248 with self.assertRaises(SystemExit) as cm:
249
250=== modified file 'cli/ci_cli/tests/test_image.py'
251--- cli/ci_cli/tests/test_image.py 2014-06-11 02:56:45 +0000
252+++ cli/ci_cli/tests/test_image.py 2014-07-04 03:45:58 +0000
253@@ -24,7 +24,7 @@
254 from ci_cli.image import ImageObjectNotFound
255 from ci_cli.tests import (
256 MainScriptTestCase,
257- FakeRequestsReturn,
258+ FakeResponse,
259 )
260
261
262@@ -83,9 +83,9 @@
263 # the image file is downloaded and stored in the specified
264 # path.
265 mock_requests.side_effect = [
266- FakeRequestsReturn(200, json.dumps(artifact_data)),
267- FakeRequestsReturn(200, json.dumps(artifact_tempurls)),
268- FakeRequestsReturn(200, 'IMAGE_CONTENT'),
269+ FakeResponse(200, json.dumps(artifact_data)),
270+ FakeResponse(200, json.dumps(artifact_tempurls)),
271+ FakeResponse(200, 'IMAGE_CONTENT'),
272 ]
273 args = self.cli.parse_arguments(
274 ['get_image', '-t', '4', '-n', self.image_path])
275@@ -102,8 +102,8 @@
276 # If the corresponding tempurl for a registered image cannot
277 # be found ImageObjectNotFound() is raised.
278 mock_requests.side_effect = [
279- FakeRequestsReturn(200, json.dumps(artifact_data)),
280- FakeRequestsReturn(200, json.dumps({})),
281+ FakeResponse(200, json.dumps(artifact_data)),
282+ FakeResponse(200, json.dumps({})),
283 ]
284 args = self.cli.parse_arguments(
285 ['get_image', '-t', '4', '-n', self.image_path])
286@@ -131,10 +131,10 @@
287 # If a ticket number is not specified, CLI looks for the
288 # latest COMPLETED ticket and downloads its image.
289 mock_requests.side_effect = [
290- FakeRequestsReturn(200, json.dumps(ticket_data)),
291- FakeRequestsReturn(200, json.dumps(artifact_data)),
292- FakeRequestsReturn(200, json.dumps(artifact_tempurls)),
293- FakeRequestsReturn(200, 'IMAGE_CONTENT'),
294+ FakeResponse(200, json.dumps(ticket_data)),
295+ FakeResponse(200, json.dumps(artifact_data)),
296+ FakeResponse(200, json.dumps(artifact_tempurls)),
297+ FakeResponse(200, 'IMAGE_CONTENT'),
298 ]
299 args = self.cli.parse_arguments(
300 ['get_image', '-n', self.image_path])
301@@ -146,7 +146,7 @@
302 # If no ticket is specified and there are no COMPLETED tickets
303 # the CLI renders a message indicating the problem.
304 mock_requests.side_effect = [
305- FakeRequestsReturn(200, json.dumps({'objects': []})),
306+ FakeResponse(200, json.dumps({'objects': []})),
307 ]
308 args = self.cli.parse_arguments(
309 ['get_image', '-n', self.image_path])
310
311=== modified file 'cli/ci_cli/tests/test_ticket.py'
312--- cli/ci_cli/tests/test_ticket.py 2014-07-01 03:48:44 +0000
313+++ cli/ci_cli/tests/test_ticket.py 2014-07-04 03:45:58 +0000
314@@ -19,7 +19,7 @@
315 import unittest
316
317 from ci_cli.tests import (
318- FakeRequestsReturn,
319+ FakeResponse,
320 MainScriptTestCase,
321 )
322 from ci_cli.ticket import (
323@@ -44,7 +44,7 @@
324 spu = SourcePackageUpload('1', 'http://server/')
325 self.assertIsNone(spu.sandbox_url)
326 mocked_post.side_effect = [
327- FakeRequestsReturn(404, 'Zoing!')
328+ FakeResponse(404, 'Zoing!')
329 ]
330 with self.assertRaises(SourcePackageUploadError) as cm:
331 spu.create()
332@@ -60,7 +60,7 @@
333 spu = SourcePackageUpload('1', 'http://server/')
334 self.assertIsNone(spu.sandbox_url)
335 mocked_post.side_effect = [
336- FakeRequestsReturn(201, '/ticket/1/sandbox/UUID')
337+ FakeResponse(201, '/ticket/1/sandbox/UUID')
338 ]
339 spu.create()
340 self.assertEquals(
341@@ -82,14 +82,14 @@
342 # `upload` raises an error if it cannot get a tempurl from GK
343 # of cannot PUT on the swift tempurl.
344 mocked_post.side_effect = [
345- FakeRequestsReturn(201, '/ticket/1/sandbox/UUID'),
346+ FakeResponse(201, '/ticket/1/sandbox/UUID'),
347 ]
348 spu = SourcePackageUpload('1', 'http://server/')
349 spu.create()
350 mocked_put.side_effect = [
351- FakeRequestsReturn(404, 'Not Found'),
352- FakeRequestsReturn(200, 'A_TEMP_URL'),
353- FakeRequestsReturn(401, 'Not Authorized'),
354+ FakeResponse(404, 'Not Found'),
355+ FakeResponse(200, 'A_TEMP_URL'),
356+ FakeResponse(401, 'Not Authorized'),
357 ]
358 with self.assertRaises(SourcePackageUploadError) as cm:
359 spu.upload('not_found_filename')
360@@ -111,13 +111,13 @@
361 # `upload` returns the GK response content (created object
362 # sandbox path) when it works.
363 mocked_post.side_effect = [
364- FakeRequestsReturn(201, '/ticket/1/sandbox/UUID'),
365+ FakeResponse(201, '/ticket/1/sandbox/UUID'),
366 ]
367 spu = SourcePackageUpload('1', 'http://server/')
368 spu.create()
369 mocked_put.side_effect = [
370- FakeRequestsReturn(200, 'A_TEMP_URL'),
371- FakeRequestsReturn(201, '/a_sandbox_object_url/'),
372+ FakeResponse(200, 'A_TEMP_URL'),
373+ FakeResponse(201, '/a_sandbox_object_url/'),
374 ]
375 mocked_of.return_value = 'a_content'
376 result = spu.upload('a_file')
377@@ -138,8 +138,8 @@
378 # `process` raises `SourcePackageUploadError` with the GK
379 # response content (upload remote validation error).
380 mocked_post.side_effect = [
381- FakeRequestsReturn(201, '/ticket/1/sandbox/UUID'),
382- FakeRequestsReturn(400, 'Problem!'),
383+ FakeResponse(201, '/ticket/1/sandbox/UUID'),
384+ FakeResponse(400, 'Problem!'),
385 ]
386 spu = SourcePackageUpload('1', 'http://server/')
387 spu.create()
388@@ -153,8 +153,8 @@
389 # `process` returns the GK response content (upload validation
390 # results) when it works.
391 mocked_post.side_effect = [
392- FakeRequestsReturn(201, '/ticket/1/sandbox/UUID'),
393- FakeRequestsReturn(201, 'Success!'),
394+ FakeResponse(201, '/ticket/1/sandbox/UUID'),
395+ FakeResponse(201, 'Success!'),
396 ]
397 spu = SourcePackageUpload('1', 'http://server/')
398 spu.create()
399@@ -168,12 +168,12 @@
400 # Using `SourcePackageUpload` as a context manager makes your
401 # life easier!
402 mocked_post.side_effect = [
403- FakeRequestsReturn(201, '/ticket/1/sandbox/UUID'),
404- FakeRequestsReturn(201, 'Success!'),
405+ FakeResponse(201, '/ticket/1/sandbox/UUID'),
406+ FakeResponse(201, 'Success!'),
407 ]
408 mocked_put.side_effect = [
409- FakeRequestsReturn(200, 'A_TEMP_URL'),
410- FakeRequestsReturn(201, '/a_sandbox_object_url/'),
411+ FakeResponse(200, 'A_TEMP_URL'),
412+ FakeResponse(201, '/a_sandbox_object_url/'),
413 ]
414 mocked_of.return_value = 'a_content'
415
416
417=== modified file 'cli/ci_cli/tests/test_utils.py'
418--- cli/ci_cli/tests/test_utils.py 2014-06-19 23:38:34 +0000
419+++ cli/ci_cli/tests/test_utils.py 2014-07-04 03:45:58 +0000
420@@ -17,10 +17,12 @@
421
422 import json
423 import mock
424+import unittest
425
426 from ci_cli import utils
427-from ci_cli.tests import FakeRequestsReturn
428+from ci_cli.tests import FakeResponse
429 from ci_utils.testing import TestCaseWithGnupg
430+from requests import exceptions
431
432
433 config_contents = """auth_url: http://example.com
434@@ -32,6 +34,62 @@
435 """
436
437
438+class TestBasicRequest(unittest.TestCase):
439+
440+ @mock.patch('requests.get')
441+ def test_get(self, mocked_get):
442+ mocked_get.side_effect = [FakeResponse(200, '{"foo": ["bar", "baz"]}')]
443+ result = utils.get("foo")
444+ self.assertEquals(result, {u'foo': [u'bar', u'baz']})
445+ mocked_get.assert_called_once()
446+
447+ @mock.patch('requests.get')
448+ def test_get_fails(self, mocked_get):
449+ mocked_get.side_effect = [FakeResponse(502, {}, "Failed get()!")]
450+ with self.assertRaises(exceptions.HTTPError) as cm:
451+ utils.get("foo")
452+ mocked_get.assert_called_once()
453+ self.assertEquals(
454+ cm.exception.message, "502 Server Error: Failed get()!")
455+
456+ @mock.patch('requests.get')
457+ def test_get_returns_invalid_content(self, mocked_get):
458+ mocked_get.side_effect = [FakeResponse(200, "Ohh, this is invalid")]
459+ with self.assertRaises(ValueError) as cm:
460+ utils.get("foo")
461+ mocked_get.assert_called_once()
462+ self.assertEquals(
463+ cm.exception.message, "No JSON object could be decoded")
464+
465+ @mock.patch('requests.patch')
466+ def test_patch(self, mocked_patch):
467+ utils.patch("foo", {}, {})
468+ mocked_patch.assert_called_once()
469+
470+ @mock.patch('requests.patch')
471+ def test_patch_fails(self, mocked_patch):
472+ mocked_patch.side_effect = [FakeResponse(502, {}, "Failed patch()!")]
473+ with self.assertRaises(exceptions.HTTPError) as cm:
474+ utils.patch("foo", {}, {})
475+ mocked_patch.assert_called_once()
476+ self.assertEquals(
477+ cm.exception.message, "502 Server Error: Failed patch()!")
478+
479+ @mock.patch('requests.post')
480+ def test_post(self, mocked_post):
481+ utils.post("foo", {}, {})
482+ mocked_post.assert_called_once()
483+
484+ @mock.patch('requests.post')
485+ def test_post_fails(self, mocked_post):
486+ mocked_post.side_effect = [FakeResponse(502, {}, "Failed post()!")]
487+ with self.assertRaises(exceptions.HTTPError) as cm:
488+ utils.post("foo", {}, {})
489+ mocked_post.assert_called_once()
490+ self.assertEquals(
491+ cm.exception.message, "502 Server Error: Failed post()!")
492+
493+
494 class UtilsTestCase(TestCaseWithGnupg):
495 def test_parse_id(self):
496 location = 'http://www.example.com/api/v1/ticket/5/'
497@@ -50,7 +108,7 @@
498 def test_get_sourcepackage_uri(self, mock_requests):
499 data = {"objects": [{"resource_uri": "/api/v1/sourcepackage/4/"}]}
500 mock_requests.side_effect = [
501- FakeRequestsReturn(200, json.dumps(data))
502+ FakeResponse(200, json.dumps(data))
503 ]
504 uri = utils.get_sourcepackage_uri('foobar')
505 mock_requests.assert_called_once()
506@@ -60,7 +118,7 @@
507 def test_get_sourcepackage_uri_not_found(self, mock_requests):
508 data = {"objects": []}
509 mock_requests.side_effect = [
510- FakeRequestsReturn(200, json.dumps(data))
511+ FakeResponse(200, json.dumps(data))
512 ]
513 uri = utils.get_sourcepackage_uri('foobar')
514 mock_requests.assert_called_once()
515
516=== modified file 'cli/ci_cli/ticket.py'
517--- cli/ci_cli/ticket.py 2014-07-01 03:43:40 +0000
518+++ cli/ci_cli/ticket.py 2014-07-04 03:45:58 +0000
519@@ -50,6 +50,10 @@
520 """Raised where there is an error during the SPU."""
521
522
523+class QueueingTicketError(Exception):
524+ """Raised when ticket failed to be set to Queued."""
525+
526+
527 class SourcePackageUpload(object):
528 """Context manager helper for performing SPUs via Gatekeeper."""
529
530@@ -145,18 +149,18 @@
531 return ''
532
533 def _parse_changes(self, changes_filepath):
534- log.info("Parsing %s..." % changes_filepath)
535+ log.info("Parsing {}...".format(os.path.basename(changes_filepath)))
536 changes = ChangesProcessor(
537 changes_filepath=changes_filepath)
538- log.info("Validating .changes file...")
539+ log.info("Validating .changes...")
540 self.files = changes.process()
541 self.version = changes.source_version
542 self.suite = changes.suite
543 self.series = self.suite.split("-")[0]
544 self.sourcepackage = changes.source_package_name
545 self.version = changes.source_version
546- log.info("%s_%s parsed and validated." % (self.sourcepackage,
547- self.version))
548+ log.info("{}_{} parsed and validated.".format(
549+ self.sourcepackage, self.version))
550
551 def _process(self):
552 self._create_spu()
553@@ -253,9 +257,16 @@
554 "status": TicketWorkflowStepStatus.QUEUED.value,
555 }
556 urlbase = utils.CI_URL + utils.API_URL + 'updateticketstatus/'
557- utils.patch(urlbase + self.ticket_id + '/', data=data)
558- print("You have successfully submitted a ticket to the Ubuntu CI"
559- " Engine. Your ticket number is %s." % self.ticket_id)
560+ try:
561+ utils.patch(urlbase + self.ticket_id + '/', data=data)
562+ print("You have successfully submitted a ticket to the Ubuntu CI "
563+ "Engine. Your ticket number is %s." % self.ticket_id)
564+ except Exception as exc:
565+ raise QueueingTicketError(
566+ "Ticket was created successfully but there was a problem "
567+ "adding it to the queue: {}. This should be a transient "
568+ "error, please contact an admin. Your ticket number is "
569+ "{}.".format(exc, self.ticket_id))
570
571 def add_new_ticket(self, args):
572 suites = []
573
574=== modified file 'cli/ci_cli/utils.py'
575--- cli/ci_cli/utils.py 2014-06-20 14:14:15 +0000
576+++ cli/ci_cli/utils.py 2014-07-04 03:45:58 +0000
577@@ -42,19 +42,21 @@
578
579
580 def post(url, data, headers=HEADERS, patch=False):
581- req = requests.post(url=url, data=json.dumps(data), headers=HEADERS)
582- resp = req.headers
583- return resp['location']
584+ response = requests.post(url=url, data=json.dumps(data), headers=HEADERS)
585+ response.raise_for_status()
586+ headers = response.headers
587+ return headers['location']
588
589
590 def patch(url, data, headers=HEADERS):
591- requests.patch(url=url, data=json.dumps(data), headers=HEADERS)
592+ response = requests.patch(url=url, data=json.dumps(data), headers=HEADERS)
593+ response.raise_for_status()
594
595
596 def get(url):
597- r = requests.get(url)
598- r.raise_for_status()
599- return r.json()
600+ response = requests.get(url)
601+ response.raise_for_status()
602+ return response.json()
603
604
605 def parse_id(location):
606
607=== modified file 'cli/ubuntu-ci'
608--- cli/ubuntu-ci 2014-07-01 03:43:40 +0000
609+++ cli/ubuntu-ci 2014-07-04 03:45:58 +0000
610@@ -184,7 +184,8 @@
611 else:
612 log.error(error)
613 except (ticket.SourcePackageUploadError,
614- ticket.MismatchSuitesError) as exc:
615+ ticket.MismatchSuitesError,
616+ ticket.QueueingTicketError) as exc:
617 log.error(exc.message)
618 except Exception:
619 log.exception('Unexpected exception')

Subscribers

People subscribed via source and target branches