Merge lp://qastaging/~doanac/uci-engine/pub-pause into lp://qastaging/uci-engine

Proposed by Andy Doan
Status: Needs review
Proposed branch: lp://qastaging/~doanac/uci-engine/pub-pause
Merge into: lp://qastaging/uci-engine
Diff against target: 126 lines (+70/-1)
5 files modified
ci-utils/ci_utils/amqp_utils.py (+13/-0)
lander/bin/lander_process_ticket.py (+5/-0)
lander/bin/lander_service_wrapper.py (+1/-1)
lander/lander/tests/test_process_ticket.py (+19/-0)
lander/lander/tests/test_service_wrapper.py (+32/-0)
To merge this branch: bzr merge lp://qastaging/~doanac/uci-engine/pub-pause
Reviewer Review Type Date Requested Status
Francis Ginther Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+223414@code.qastaging.launchpad.net

Commit message

lander: allow a worker to request a 'retry'

Workers like the publisher need to mark a ticket step as needing
to be retried. This adds a function, amqp_utils.progress_retry. When
called by a worker, the lander will pause the ticket and wait until
the ticket is updated to "resume". At this point the lander will resume
execution at this step and see if it can progress

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

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

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

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

Hi Andy,

I've asked some questions on IRC a while ago but I think they got lost -- I should've asked them here, sorry about that.

Revision history for this message
Joe Talbott (joetalbott) wrote :

This looks okay to me code-wise. I'll leave this MP to you and Ursula to sort out as far as her comments are concerned.

Revision history for this message
Andy Doan (doanac) wrote :

I've responded to Ursula. I'd say we keep it the way I have it, but I'll let you guys decide.

Revision history for this message
Francis Ginther (fginther) wrote :

I think this does what it's intended to do (and there are unit tests to match). The progress_retry() isn't being used anywhere, so this shouldn't regress anything.

Only question is if the name 'progress_retry' was meant to be named 'progress_pause'?

I'll approve as we really can't do much more with it until something else is in place to exercise it.

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

On Thu, Jul 17, 2014 at 10:32 PM, Francis Ginther <
<email address hidden>> wrote:

> > +def progress_retry(progress_trigger, data=None):
> > + '''Let the engine know the ticket should be paused and retried
> after some
> > + manual acknowledgement has taken place
> > + '''
> > + if data is None:
> > + data = {}
> > + data['state'] = 'RETRY'
>
> It sounds a bit odd to have this method called 'progress_retry' when the
> outcome is to pause the workflow. I think this is what Ursula is pointing
> out.
>

Ah - i get it now. So "retry" in this since is still a manual thing that
takes advantage of our internal "pause" support. ie - the way the step
would get repeated would be for some interaction that puts the ticket
status into "continue"

Unmerged revisions

589. By Andy Doan

lander: allow a worker to request a 'retry'

Workers like the publisher need to mark a ticket step as needing
to be retried. This adds a function, amqp_utils.progress_retry. When
called by a worker, the lander will pause the ticket and wait until
the ticket is updated to "resume". At this point the lander will resume
execution at this step and see if it can progress

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