daf_persistence add functions to query if a storage exists at a location #50
Conversation
| bool | ||
| True if the storage exists, false if not | ||
| """ | ||
| return os.path.exists(PosixStorage._pathFromURI(uri)) |
There was a problem hiding this comment.
It looks like this boils down to a stat(2) call, so this should be nice and atomic.
There was a problem hiding this comment.
I believe @timj's concern is valid. Two threads could both make the stat(2) call while the dir does not exist and then collide on directory creation.
|
This looks reasonable to me but I am unfamiliar with this code. |
| @@ -50,7 +50,7 @@ def __init__(self, uri): | |||
| :return: | |||
| """ | |||
| self.log = Log.getLogger("daf.persistence.butler") | |||
| self.root = urllib.parse.urlparse(uri).path | |||
| self.root = self._pathFromURI(uri) | |||
| if self.root and not os.path.exists(self.root): | |||
| os.makedirs(self.root) | |||
There was a problem hiding this comment.
I think this should use the safe safeMakeDir().
There was a problem hiding this comment.
I missed this... is using the exist_ok param not enough?
There was a problem hiding this comment.
I looked at safeMakeDir and I'm not sure if it is actually safe if the path it is given contains multiple [new] nested dirs.
There was a problem hiding this comment.
I'm not familiar with the code much and the code looks fine as such but I have a big BUT. Can you please explain in the commit message why this is fixing the race condition described in DM-9848? On the face of it the posixStorage code has just moved the URL parsing from one place to another without obviously adjusting how the os.makedirs is run.
| @@ -601,6 +606,22 @@ def search(root, path, searchParents=False): | |||
| else: | |||
| return None | |||
|
|
|||
| @staticmethod | |||
| def storageExists(uri): | |||
There was a problem hiding this comment.
This method is added but nothing in this commit seems to call it. Is it a specialization of a base implementation?
There was a problem hiding this comment.
it's called by the new code in storageContinued.py
No description provided.