call update_engine_client and wait on exit code.

call update_engine_client and wait on exit code.

Change-Id: I54fd8f30c391cd44f25e827883628484c22716ab

[ericli, 2010-08-23 16:19:33]

LGTM.

[petkov, 2010-08-23 19:20:31]

A bit late, I guess...

LGTM too with some suggestions for making the update code more robust.

http://codereview.chromium.org/3175034/diff/1/2
File client/common_lib/chromiumos_updater.py (right):

http://codereview.chromium.org/3175034/diff/1/2#newcode45
client/common_lib/chromiumos_updater.py:45: self._run('initctl stop
update-engine')
Does this throw an exception if update-engine is not running already?

http://codereview.chromium.org/3175034/diff/1/2#newcode78
client/common_lib/chromiumos_updater.py:78: if self.check_update_status() !=
UPDATER_IDLE:
This may throw an exception if update-engine is not started for some reason.
Maybe catch it so that we still try to reset update_engine? Either here, or
inside check_update_status?

http://codereview.chromium.org/3175034/diff/1/2#newcode87
client/common_lib/chromiumos_updater.py:87: '--app_version ForcedUpdate',
FYI, currently --update implies --app_version=ForcedUpdate (unless app_version
is set to something else).

http://codereview.chromium.org/3175034/diff/1/2#newcode101
client/common_lib/chromiumos_updater.py:101: # TODO(seano): should we
aggressively reset update-engine here?
It might be good to somehow log /var/log/update_engine.log here so that we can
triage issues.

[seano, 2010-08-23 19:22:33]

Agreed with all comments. Will send a follow up CL to robust-ify the
update test code.

This CL makes it no *less* robust, but still has a handful of TODOs
that I'll address.

On Mon, Aug 23, 2010 at 9:20 PM,  <petkov@chromium.org> wrote:
> A bit late, I guess...
>
> LGTM too with some suggestions for making the update code more robust.
>
>
>
> http://codereview.chromium.org/3175034/diff/1/2
> File client/common_lib/chromiumos_updater.py (right):
>
> http://codereview.chromium.org/3175034/diff/1/2#newcode45
> client/common_lib/chromiumos_updater.py:45: self._run('initctl stop
> update-engine')
> Does this throw an exception if update-engine is not running already?
>
> http://codereview.chromium.org/3175034/diff/1/2#newcode78
> client/common_lib/chromiumos_updater.py:78: if
> self.check_update_status() != UPDATER_IDLE:
> This may throw an exception if update-engine is not started for some
> reason. Maybe catch it so that we still try to reset update_engine?
> Either here, or inside check_update_status?
>
> http://codereview.chromium.org/3175034/diff/1/2#newcode87
> client/common_lib/chromiumos_updater.py:87: '--app_version
> ForcedUpdate',
> FYI, currently --update implies --app_version=ForcedUpdate (unless
> app_version is set to something else).
>
> http://codereview.chromium.org/3175034/diff/1/2#newcode101
> client/common_lib/chromiumos_updater.py:101: # TODO(seano): should we
> aggressively reset update-engine here?
> It might be good to somehow log /var/log/update_engine.log here so that
> we can triage issues.
>
> http://codereview.chromium.org/3175034/show
>