Ubuntu CI Engine

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

  1. update-ticket-raises-error
  2. Merge into trunk
Proposed by Ursula Junque
Status: Merged
Approved by: Ursula Junque
Approved revision: 648
Merged at revision: 646
Proposed branch: lp://staging/~ursinha/uci-engine/update-ticket-raises-error
Merge into: lp://staging/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://staging/~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.staging.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.
lp://staging/~ursinha/uci-engine/update-ticket-raises-error updated
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.

lp://staging/~ursinha/uci-engine/update-ticket-raises-error updated
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
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches