Landscape Client

Merge lp://staging/~free.ekanayaka/landscape-client/changer-after-smart into lp://staging/~landscape/landscape-client/trunk

  1. changer-after-smart
  2. Merge into trunk
Proposed by Free Ekanayaka
Status: Merged
Approved by: Muharem Hrnjadovic
Approved revision: 217
Merged at revision: not available
Proposed branch: lp://staging/~free.ekanayaka/landscape-client/changer-after-smart
Merge into: lp://staging/~landscape/landscape-client/trunk
Diff against target: 1029 lines (+370/-83)
15 files modified
landscape/broker/broker.py (+10/-0)
landscape/broker/remote.py (+8/-0)
landscape/broker/tests/test_remote.py (+32/-0)
landscape/lib/fs.py (+7/-0)
landscape/lib/tests/test_fs.py (+27/-2)
landscape/manager/packagemanager.py (+5/-0)
landscape/manager/tests/test_packagemanager.py (+17/-0)
landscape/package/changer.py (+11/-1)
landscape/package/facade.py (+7/-2)
landscape/package/reporter.py (+51/-24)
landscape/package/taskhandler.py (+5/-0)
landscape/package/tests/test_changer.py (+26/-1)
landscape/package/tests/test_facade.py (+12/-1)
landscape/package/tests/test_reporter.py (+139/-52)
landscape/package/tests/test_taskhandler.py (+13/-0)
To merge this branch: bzr merge lp://staging/~free.ekanayaka/landscape-client/changer-after-smart
Reviewer Review Type Date Requested Status
Muharem Hrnjadovic (community) Approve
Thomas Herve (community) Approve
Review via email: mp+22601@code.staging.launchpad.net

Description of the change

This is ready for review. I've real-world tested it, and it seems to solve the problem. There's a change in SmartFacade.reload_channels that it's not strictly needed, but that traceback can help dubugging if something goes wrong.

To post a comment you must log in.
Revision history for this message
Thomas Herve (therve) wrote :

[1]
+ def changes_detected(results):
+ # Release all smart locks, in case the changer runs after us.
+ self._facade.deinit()
+ if True in results:
+ # Something has changed, notify the broker.
+ return self._broker.fire_event("package-data-changed")
+
+ result = gather_results([self.detect_packages_changes(),
+ self.detect_package_locks_changes()])
+ return result.addCallback(changes_detected)

I'm not sure if this matters, but deinit() will only be called if both operations succeed. If there is a lock, it may not be cleaned if an exception is raised.

It's a tricky change, but it looks good. Lots of testing will be involved :). +1!

review: Approve
Revision history for this message
Free Ekanayaka (free.ekanayaka) wrote :

Thanks Thomas.

We need to perform the SRU tests this week, so we can catch possible problems.

[1]

I think it's fine, if either Deferred errbacks we definitely have a bug because they're expected to handle errors.

Revision history for this message
Muharem Hrnjadovic (al-maisan) wrote :

I did not quite understand why we raise the error in check_smart_update_stamp() ("If the smart-update stamp file doesn't exist a L{PackageTaskError} is raised and the task will be picked up again at the next run.")

Other than that, this branch looks good.

review: Approve
lp://staging/~free.ekanayaka/landscape-client/changer-after-smart updated
218. By Free Ekanayaka

Fix lints (Muharem)

Revision history for this message
Free Ekanayaka (free.ekanayaka) wrote :

As said on IRC there was an unpushed revision, that PackageTaskError is gone there.

Thanks!

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: