landscape-client doesn't catch errors from smart

This also happened in staging (ls1):

2008-09-11 21:05:20,660 WARNING [MainThread] Package changer output:
Traceback (most recent call last):
  File "/usr/bin/landscape-package-changer", line 10, in <module>
    main(sys.argv[1:])
  File "/usr/lib/python2.5/site-packages/landscape/package/changer.py", line 140, in main
    return run_task_handler(PackageChanger, args)
  File "/usr/lib/python2.5/site-packages/landscape/package/taskhandler.py", line 106, in run_task_handler
    result = handler.run()
  File "/usr/lib/python2.5/site-packages/landscape/package/changer.py", line 30, in run
    result = super(PackageChanger, self).run()
  File "/usr/lib/python2.5/site-packages/landscape/package/taskhandler.py", line 29, in run
    return self.handle_tasks()
  File "/usr/lib/python2.5/site-packages/landscape/package/changer.py", line 40, in handle_tasks
    result = super(PackageChanger, self).handle_tasks()
  File "/usr/lib/python2.5/site-packages/landscape/package/taskhandler.py", line 33, in handle_tasks
    self._handle_next_task(None, deferred)
  File "/usr/lib/python2.5/site-packages/landscape/package/taskhandler.py", line 45, in _handle_next_task
    result = self.handle_task(task)
  File "/usr/lib/python2.5/site-packages/landscape/package/changer.py", line 46, in handle_task
    result = self._handle_change_packages(message)
  File "/usr/lib/python2.5/site-packages/landscape/package/changer.py", line 98, in _handle_change_packages
    result = self._facade.perform_changes()
  File "/usr/lib/python2.5/site-packages/landscape/package/facade.py", line 166, in perform_changes
    self._ctrl.commitChangeSet(changeset)
  File "/usr/lib/python2.5/site-packages/smart/control.py", line 548, in commitChangeSet
    caching)
  File "/usr/lib/python2.5/site-packages/smart/control.py", line 667, in fetchPackages
    for url in failed])
smart.Error: Failed to download packages:
    http://us.archive.ubuntu.com/ubuntu/pool/main/u/update-manager/update-manager-core_0.93.9_i386.deb: File not found

Pushed for thames-pre-9

I'm bumping this to high because I'm getting it more and more often. It's also troublesome because the UI only shows the activity in the "delivered" state, giving no hit whatsoever about a problem. And it will stay like that for all eternity, which looks bad.

Pushed for the final thames milestone.

It just happened again, in staging this time: ls2 has now a stalled activity because of this.

https://staging.landscape.canonical.com/account/landscape-devel/activity/56294

Client logs:
2008-11-20 12:12:46,681 WARNING [MainThread] Package changer output:
Traceback (most recent call last):
  File "/usr/bin/landscape-package-changer", line 10, in <module>
    main(sys.argv[1:])
  File "/usr/lib/python2.5/site-packages/landscape/package/changer.py", line 154, in main
    return run_task_handler(PackageChanger, args)
  File "/usr/lib/python2.5/site-packages/landscape/package/taskhandler.py", line 106, in run_task_handler
    result = handler.run()
  File "/usr/lib/python2.5/site-packages/landscape/package/changer.py", line 44, in run
    result = super(PackageChanger, self).run()
  File "/usr/lib/python2.5/site-packages/landscape/package/taskhandler.py", line 29, in run
    return self.handle_tasks()
  File "/usr/lib/python2.5/site-packages/landscape/package/changer.py", line 54, in handle_tasks
    result = super(PackageChanger, self).handle_tasks()
  File "/usr/lib/python2.5/site-packages/landscape/package/taskhandler.py", line 33, in handle_tasks
    self._handle_next_task(None, deferred)
  File "/usr/lib/python2.5/site-packages/landscape/package/taskhandler.py", line 45, in _handle_next_task
    result = self.handle_task(task)
  File "/usr/lib/python2.5/site-packages/landscape/package/changer.py", line 60, in handle_task
    result = self._handle_change_packages(message)
  File "/usr/lib/python2.5/site-packages/landscape/package/changer.py", line 112, in _handle_change_packages
    result = self._facade.perform_changes()
  File "/usr/lib/python2.5/site-packages/landscape/package/facade.py", line 166, in perform_changes
    self._ctrl.commitChangeSet(changeset)
  File "/usr/lib/python2.5/site-packages/smart/control.py", line 548, in commitChangeSet
    caching)
  File "/usr/lib/python2.5/site-packages/smart/control.py", line 667, in fetchPackages
    for url in failed])
smart.Error: Failed to download packages:
    http://security.ubuntu.com/ubuntu/pool/main/h/hplip/hpijs_2.7.7+2.7.7.dfsg.1-0ubuntu5.1_i386.deb: Unexpected size (expected 334064, got 334576)

This keeps happening, and the activity in Landscape stays "Delivered" forever.

The problem occurs when Smart (for whatever reason) was unable to download a certain file, and consequently raises a smart.Error exception.

We now properly catch the exception:

- self._ctrl.commitChangeSet(changeset)
+ try:
+ self._ctrl.commitChangeSet(changeset)
+ except smart.Error, e:
+ raise TransactionError(e.args[0])

and the PackageChanger will send an error message with information about the failure, as it happens with other errors.

[1]

+ """
+ @param task: A L{PackageTask} carrying a message of
+ type C{"change-packages"}.
+ """

Would you mind formatting this, and other similar cases like this
(ie, follow on lines are indented only 4-spaces):

        """
        @param task: A L{PackageTask} carrying a message of
             type C{"change-packages"}.
        """

This is the predominant way we've formatted API properties in the
code base, so it'd be nice to keep that consistent.

This looks great. I'm not sure how to test this for real, so I've
just reviewed the code and run the tests. +1.

Looks good! +1

Thanks Jamu!

[1]

Fix in r95. I believe there are a few ways to test the fix for real. For instance have the APT sources on the client point to some local mirror, physically remove a deb from the mirror, and then try to install right that removed package. Smart should fail with something like:

smart.Error: Failed to download packages:
    http://us.archive.ubuntu.com/ubuntu/pool/main/u/update-manager/update-manager-core_0.93.9_i386.deb: File not found

(see one of the reports above).

It really seems to be fixed :)

http://www.ubuntu-pics.de/bild/11828/failed_download_Z80May.png

I'm going to merge it into the trunk.

This bug was fixed in the package landscape-client - 1.0.29-0ubuntu0.9.04.0

---------------
landscape-client (1.0.29-0ubuntu0.9.04.0) jaunty; urgency=low

  * New upstream bugfix release (LP: #358744)
    - Add a timeout to HTTP operations to avoid hanging (LP: #349737)
    - Clean up environment variables on startup to avoid propagating
      variables that will corrupt package installation (LP: #348681)
    - Clean up FDs on startup for the same reason (LP: #352458)
    - Catch and handle certain errors from smart (such as invalid package
      data) to avoid "stuck" Landscape activities (LP: #268745)
    - Don't print warnings meant for developers to the console (LP: #336669)

 -- Christopher Armstrong <email address hidden> Thu, 09 Apr 2009 17:09:50 -0400