Merge lp:~robru/launchpadlib/fix-cache-creation into lp:launchpadlib

Proposed by Robert Bruce Park
Status: Merged
Merged at revision: 145
Proposed branch: lp:~robru/launchpadlib/fix-cache-creation
Merge into: lp:launchpadlib
Diff against target: 35 lines (+9/-2)
1 file modified
src/launchpadlib/launchpad.py (+9/-2)
To merge this branch: bzr merge lp:~robru/launchpadlib/fix-cache-creation
Reviewer Review Type Date Requested Status
Colin Watson (community) Approve
Adam Conrad Pending
LAZR Developers Pending
Review via email: mp+276561@code.launchpad.net

Description of the change

Working on a charm for porter boxes, trying to run stephane's sbuild-launchpad-chroot script a few times in parallel and discovered this race condition. Basically different processes all see the directory as not existing and then all try to create it. One wins the race and the rest lose.

To post a comment you must log in.
Revision history for this message
Dimitri John Ledkov (xnox) wrote :

Looks good. Can it be checked that the error that is eaten is in fact EEXIST?

Revision history for this message
Barry Warsaw (barry) wrote :

On Nov 03, 2015, at 04:24 PM, Dimitri John Ledkov wrote:

>Looks good. Can it be checked that the error that is eaten is in fact EEXIST?

If that's specifically what you're trying to catch, and you have to remain
Python 2.7 compatible, the idiom is:

import errno

       try:
           os.makedirs(launchpadlib_dir, 0o700)
       except OSError as error:
           if error.errno != errno.EEXIST:
               raise

Of course, we can do better in Python 3. :)

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

@barry launchpadlib must be bi-lingual for now, as not everything has yet ported over to python3.

146. By Robert Bruce Park

Re-raise if errno not EEXIST.

Revision history for this message
Robert Bruce Park (robru) wrote :

Thanks barry, yeah I agree python3 is significantly better in this regard but I don't want to be the guy who breaks py2 support in lplib just yet ;-)

Revision history for this message
Robert Bruce Park (robru) wrote :

Here's the traceback this fixes btw:

https://pastebin.canonical.com/143314/

Revision history for this message
Barry Warsaw (barry) wrote :

On Nov 03, 2015, at 04:34 PM, Dimitri John Ledkov wrote:

>@barry launchpadlib must be bi-lingual for now, as not everything has yet
>ported over to python3.

Yep. That's why I gave the bilingual compatible idiom. :)

Revision history for this message
Colin Watson (cjwatson) :
review: Approve
Revision history for this message
Robert Bruce Park (robru) wrote :

Thanks Colin!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/launchpadlib/launchpad.py'
--- src/launchpadlib/launchpad.py 2015-07-06 17:58:11 +0000
+++ src/launchpadlib/launchpad.py 2015-11-03 16:35:25 +0000
@@ -22,6 +22,7 @@
22 ]22 ]
2323
24import os24import os
25import errno
25try:26try:
26 from urllib.parse import urlsplit27 from urllib.parse import urlsplit
27except:28except:
@@ -600,8 +601,11 @@
600 if launchpadlib_dir[:1] == '~':601 if launchpadlib_dir[:1] == '~':
601 raise ValueError("Must set $HOME or pass 'launchpadlib_dir' to "602 raise ValueError("Must set $HOME or pass 'launchpadlib_dir' to "
602 "indicate location to store cached data")603 "indicate location to store cached data")
603 if not os.path.exists(launchpadlib_dir):604 try:
604 os.makedirs(launchpadlib_dir, 0o700)605 os.makedirs(launchpadlib_dir, 0o700)
606 except OSError as err:
607 if err.errno != errno.EEXIST:
608 raise
605 os.chmod(launchpadlib_dir, 0o700)609 os.chmod(launchpadlib_dir, 0o700)
606 # Determine the real service root.610 # Determine the real service root.
607 service_root = uris.lookup_service_root(service_root)611 service_root = uris.lookup_service_root(service_root)
@@ -609,6 +613,9 @@
609 scheme, host_name, path, query, fragment = urlsplit(service_root)613 scheme, host_name, path, query, fragment = urlsplit(service_root)
610 service_root_dir = os.path.join(launchpadlib_dir, host_name)614 service_root_dir = os.path.join(launchpadlib_dir, host_name)
611 cache_path = os.path.join(service_root_dir, 'cache')615 cache_path = os.path.join(service_root_dir, 'cache')
612 if not os.path.exists(cache_path):616 try:
613 os.makedirs(cache_path, 0o700)617 os.makedirs(cache_path, 0o700)
618 except OSError as err:
619 if err.errno != errno.EEXIST:
620 raise
614 return (service_root, launchpadlib_dir, cache_path, service_root_dir)621 return (service_root, launchpadlib_dir, cache_path, service_root_dir)

Subscribers

People subscribed via source and target branches