Merge lp://staging/~divmod-dev/divmod.org/start-scheduler-when-parent-service-starts-3000-2 into lp://staging/divmod.org

Proposed by Jean-Paul Calderone
Status: Merged
Approved by: Tristan Seligmann
Approved revision: 2689
Merged at revision: 2695
Proposed branch: lp://staging/~divmod-dev/divmod.org/start-scheduler-when-parent-service-starts-3000-2
Merge into: lp://staging/divmod.org
Diff against target: 330 lines (+117/-27)
6 files modified
Axiom/axiom/scheduler.py (+27/-0)
Axiom/axiom/store.py (+9/-3)
Axiom/axiom/test/historic/stub_subscheduler1to2.py (+3/-1)
Axiom/axiom/test/historic/test_subscheduler1to2.py (+10/-8)
Axiom/axiom/test/test_scheduler.py (+25/-5)
Axiom/axiom/test/test_upgrading.py (+43/-10)
To merge this branch: bzr merge lp://staging/~divmod-dev/divmod.org/start-scheduler-when-parent-service-starts-3000-2
Reviewer Review Type Date Requested Status
Tristan Seligmann Approve
Review via email: mp+87280@code.staging.launchpad.net

Description of the change

This fixes the scheduling startup issue by ensuring the scheduler powerup is created and associated with the store service as soon as the store service is created.

To post a comment you must log in.
Revision history for this message
Tristan Seligmann (mithrandi) wrote :

This all basically looks good, and fixes an annoying long-standing bug, which is great. I just have one very minor thing to mention:

62 + if getattr(scheduler, 'setServiceParent', None) is not None:
63 + scheduler.setServiceParent(collection)

It seems a bit awkward to use getattr to (conditionally) retrieve the attribute and then retrieve it again via dot syntax. Perhaps this might be better:

setServiceParent = getattr(scheduler, 'setServiceParent', None)
if setServiceParent is not None:
    setServiceParent(collection)

or even:

getattr(scheduler, 'setServiceParent', lambda ign: None)(collection)

Please go ahead and merge regardless of whether you change this code or not.

review: Approve
2690. By Jean-Paul Calderone

Avoid an extra attribute lookup

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

to all changes: