Landscape Client

Merge lp://staging/~tealeg/landscape-client/fix-double-launch into lp://staging/~landscape/landscape-client/trunk

  1. fix-double-launch
  2. Merge into trunk
Proposed by Geoff Teale
Status: Superseded
Proposed branch: lp://staging/~tealeg/landscape-client/fix-double-launch
Merge into: lp://staging/~landscape/landscape-client/trunk
Diff against target: 38 lines (+14/-4)
2 files modified
landscape/ui/model/registration/proxy.py (+1/-1)
scripts/landscape-client-settings-ui (+13/-3)
To merge this branch: bzr merge lp://staging/~tealeg/landscape-client/fix-double-launch
Reviewer Review Type Date Requested Status
Thomas Herve (community) Needs Fixing
Alberto Donato Approve
Review via email: mp+98453@code.staging.launchpad.net

This proposal has been superseded by a proposal from 2012-03-21.

Description of the change

This fixes #960211

There are two issues here, firstly the traceback itself (the call to disable_error should actually be to disable_fail and have no message) and secondly the underlying cause of the error: it was possible to launch the UI twice, under which circumstances you could end up taking actions with an inconsistant data set.

To post a comment you must log in.
Revision history for this message
Alberto Donato (ack) wrote :

Looks good, +1!

#1:
please use double quotes instead of single

#2:
+ this_pid = os.getpid()

this can go outside of the for loop

#3:
+ sys.stderr.flush()

I don't think you need to explicitly flush stderr, it's unbuffered. It doesn't make any harm, though.

review: Approve
Revision history for this message
Thomas Herve (therve) wrote :

[1] It'd be nicer to use a lock to be sure we have one running.

review: Needs Fixing
lp://staging/~tealeg/landscape-client/fix-double-launch updated
533. By Geoff Teale

Use landscape.lib.lock instead of scanning /proc (From therve's review.

Revision history for this message
Geoff Teale (tealeg) wrote :

> [1] It'd be nicer to use a lock to be sure we have one running.

Good call, done!

Revision history for this message
Geoff Teale (tealeg) wrote :

> Looks good, +1!
>
> #1:
> please use double quotes instead of single
>
> #2:
> + this_pid = os.getpid()
>
> this can go outside of the for loop
>
> #3:
> + sys.stderr.flush()
>
> I don't think you need to explicitly flush stderr, it's unbuffered. It doesn't
> make any harm, though.

Addressing therve's comment actually removed the code that you are referring to, so essentially these comments are addressed.

lp://staging/~tealeg/landscape-client/fix-double-launch updated
534. By Geoff Teale

Add try/finallny block to ensure we always unlock (both therve and ack's reviews).

Unmerged revisions

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: