Merge lp:~dobey/ubuntuone-client/prefs-crashing into lp:ubuntuone-client

Proposed by dobey on 2009-10-05
Status: Merged
Approved by: Tim Cole on 2009-10-06
Approved revision: 234
Merged at revision: not available
Proposed branch: lp:~dobey/ubuntuone-client/prefs-crashing
Merge into: lp:ubuntuone-client
Diff against target: 12 lines
1 file modified
bin/ubuntuone-client-preferences (+3/-0)
To merge this branch: bzr merge lp:~dobey/ubuntuone-client/prefs-crashing
Reviewer Review Type Date Requested Status
Tim Cole (community) Approve on 2009-10-06
Guillermo Gonzalez 2009-10-05 Approve on 2009-10-06
Review via email:

Commit message

Create the config directory if it doesn't exist yet, in prefs dialog

To post a comment you must log in.
review: Approve
Tim Cole (tcole) wrote :

Looks okay. As a general principle though, I really prefer to see file/directory creation tried blindly and any EEXIST error caught, since that way if e.g. two instances are started around the same time one of them won't blow up due to a race between the existence test and creation.

review: Approve
Tim Cole (tcole) wrote :

Basically, when a creation is non-idempotent and would fail if the resource already exists, it's much better to ask forgiveness than permission.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/ubuntuone-client-preferences'
2--- bin/ubuntuone-client-preferences 2009-09-25 18:48:03 +0000
3+++ bin/ubuntuone-client-preferences 2009-10-05 19:40:23 +0000
4@@ -105,6 +105,9 @@
6 def load_config(self):
7 """Load the configuration, and initilize if empty."""
8+ if not os.path.isdir(os.path.dirname(CONF_FILE)):
9+ os.makedirs(os.path.dirname(CONF_FILE))
11 self.config = ConfigParser()


People subscribed via source and target branches