Merge lp://staging/~jderose/filestore/explicit-create into lp://staging/filestore

Proposed by Jason Gerard DeRose
Status: Merged
Merged at revision: 320
Proposed branch: lp://staging/~jderose/filestore/explicit-create
Merge into: lp://staging/filestore
Diff against target: 3188 lines (+1996/-623)
9 files modified
benchmark-protocol.py (+23/-42)
debian/control (+2/-2)
filestore/__init__.py (+230/-174)
filestore/migration.py (+281/-0)
filestore/misc.py (+16/-8)
filestore/tests/__init__.py (+816/-388)
filestore/tests/run.py (+5/-1)
filestore/tests/test_migration.py (+548/-0)
filestore/tests/test_misc.py (+75/-8)
To merge this branch: bzr merge lp://staging/~jderose/filestore/explicit-create
Reviewer Review Type Date Requested Status
James Raymond Approve
Review via email: mp+163821@code.staging.launchpad.net

Description of the change

For background, see this bug:

  https://bugs.launchpad.net/filestore/+bug/1163002

The are two interwoven goals here. The first is to force the programmer always to choose between assuming no file-store yet exists in a given parentdir, or assuming a file-store already exists in a given parentdir, with an unforgiving mountain of exceptions and double checks to punish anyone who crosses the streams. Architecturally the previous fuzzy gray area here was a really bad call on my part, and so I'm trying to atone now =)

The second is to cleanly separate out the V0 to V1 upgrade functionality from day to day FileStore functionality. Because the act of creating a FileStore instance could implicitly initialize a file-store layout where there wasn't one before, the upgrade functionality really had to be hooked into FileStore.__init__() and was kind of a mess. But as this gray area has been vanquished, it's now easy to isolate the upgrade functionality from FileStore.__init__().

It's better to do the "soft-upgrade" (basically, moving files/ to files0/) before you create a FileStore instance, which you can now do:

>>> parentdir = '/media/jderose/BigDrive'
>>> m = Migration(parentdir)
>>> m.needs_upgrade() # Soft upgrade if needed, but wont "create" anything
>>> fs = FileStore(parentdir) # Assumes the file-store layout already exists
>>> fs = FileStore.create(parentdir) # The alternative assumption, it does not exist

If `Migration.needs_upgrade()` returns True, then you (likely) have files that need to be re-hashed in order to migrate them from the V0 to the V1 IDs, which you can do by running `dmedia-v0-v1-upgrade`. This can be very time consuming, but can be resumed from where you left off (and the intermediate result will even be synced between your devices).

Anyway, changes include:

* A file-store layout now must always be explicitly initialized, which is done using the new FileStore.create() classmethod:

   FileStore.create(parentdir, store_id=None, copies=1, **kw)

   If path.join(parentdir, '.dmedia') already exists, this method will fail loudly. A big difference between this and the old FileStore.init_dirs() is this classmethod assigns the FileStore an ID (which you can manually provide via *store_id*) and also writes out the "document" to a JSON file, which for an easier migration from V0 is now in 'filestore.json' instead of 'store.json'. So FileStore.create() is taking responsibility for this part rather than letting Dmedia handle it.

   This is a slightly different API than originally discussed in the bug, but there are a few circumstances where manually assigned the ID is handy. For one, unit testing, like to see what happens if you try to connect two physically distinct file-stores that happen to have the same ID. Also, for performance reasons one might want to periodically format a drive, but then initialize the file-store using the previous ID so the ID is stable over time.

   Any additional **kw get included in the filestore.json document. The two that seem handy to me at the moment are `uuid` and `label` (the containing partition's UUID and Label).

* On the other hand, FileStore.__init__() will now fail if the file-store layout does *not* already exist, and has a slightly different signature:

   FileStore.__init__(parentdir, expected_id=None)

   Note that when "loading" an existing file-store, you can't actually change the ID. The optional `expected_id` kwarg tells FileStore.__init__() to *fail* if the ID in filestore.json doesn't match `expected_id`. This is mostly for subprocesses so that they have a more robust way to stay on the same page with the parent process in terms of the currently connect file-stores.

   Also note that previously you could sort of "override" the copies value, but now this can only be provided in FileStore.create(). FileStore.__init__() simply loads these values from filestore.json.

* Added the low-level `_create_filestore()` function that both `FileStore.create()` and `TempFileStore.__init__()` use:

   _create_filestore(parentdir, store_id, copies, **kw)

   It isn't indented as part of the public API just yet, but I think it's a big step forward in terms of the file-store layout initialization being closer to atomic. It's a lot more robust than it was. It creates the layout in temporary directory .dmedia-<STORE_ID>, does everything relative to an open directory descriptor (hooray for Python 3.3), and then at the very last minute renames:

   .dmedia-<STORE_ID> => .dmedia

* FileStore instances (including TempFileStore) now all have a FileStore.doc attribute, which is a Python dict and is the exact Dmedia V1 scheme compliant doc ready to save into CouchDB. This doc is dumped to filestore.json by FileStore.create(), and then is loaded whenever you load an existing file-store with FileStore.__init__()

* Moved all upgrade/migration functionality out of filestore and into the new filestore.migration module; this is much more robust than before and has extensive unit tests

* A bit of sundry cleanup and modernization, of the sort I just can't turn down during a refactor-fest such as this

To post a comment you must log in.
Revision history for this message
James Raymond (jamesmr) :
review: Approve

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