Merge lp://qastaging/~doanac/ubuntu-ci-services-itself/publisher-ppa-sync into lp://qastaging/ubuntu-ci-services-itself

ppa-assigner/ppa_assigner/api.py still has:
  from ppa_assigner import ppa_sync

This breaks startup of ppa_django. Still reviewing.

68-95: the try block in lander/bin/lander_service_wrapper.py:

There are two actions in this try block, the ppa_copy and the artifact creation. If either fails, the publiser step is marked as failed. I believe this is the right outcome and is consistent with other components. Another thing I considered if either of these actions could be retried if it fails and I don't see a use case where it would help.

So I thinks is right, but please give it a second thought if I overlooked something.

I'm finished reviewing this.

On 03/16/2014 01:51 PM, Francis Ginther wrote:
> 68-95: the try block in lander/bin/lander_service_wrapper.py:
>
> There are two actions in this try block, the ppa_copy and the
> artifact creation. If either fails, the publiser step is marked as
> failed. I believe this is the right outcome and is consistent with
> other components. Another thing I considered if either of these
> actions could be retried if it fails and I don't see a use case where
> it would help.

Hmm. I think you've noticed something here. Really once we are through
that first block, the ticket has been marked COMPLETE. The 2nd block is
merely uploading/attaching artifacts. I don't think its worth failing an
otherwise successfull ticket over that. That should be easy to fix.

I'm testing revno 404 right now with:

 http://15.125.119.88/ticket.html?ticket_id=4

Not sure I totally trust myself patching this on the fly like I am

ticket: http://15.125.119.88/ticket.html?ticket_id=4 looks good. moving to "needs review"

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

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

90 + with open(output) as f:
91 + ticket_id = config['master']['request_id']
92 + auth = unit_config.get_auth_config()
93 + ds = data_store.create_for_ticket(ticket_id, auth)
94 + url = ds.put_file(output, f.read(), 'text/plain')
95 + result['artifacts'] = [{
96 + 'name': output,
97 + 'reference': url,
98 + 'type': 'LOGS',
99 + }]
100 + ticket.process_artifacts(result)

This is now outside the try block. Is that what you want? If it fails, the exception will now be caught by the try block in main(). This looks right to me as it's still a failure that needs to be surfaced and dealt with (in this case it would behave in a similar way as if the API call had failed). I'll approve, but just want to mention this in case you had a different outcome in mind.

On 03/17/2014 10:44 AM, Francis Ginther wrote:
> This is now outside the try block. Is that what you want? If it
> fails, the exception will now be caught by the try block in main().
> This looks right to me as it's still a failure that needs to be
> surfaced and dealt with (in this case it would behave in a similar
> way as if the API call had failed). I'll approve, but just want to
> mention this in case you had a different outcome in mind.

It fails the Jenkins job, but not the ticket. I figured this was a good
distinction. ie - there was a failure we might want to investigate. Its
just not a failure that should stop a ticket from being complete.