Require explicit FileStore creation

Bug #1163002 reported by Jason Gerard DeRose on 2013-04-01
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
FileStore
High
Jason Gerard DeRose

Bug Description

So the time has come to make the V1 protocol the default, and to add the needed V0 => V1 migration functionality.

As this is a disruptive change that will need to land at the same time as the equivalent change in Dmedia, I've given some thought to other breaking API change we might want to do.

And there is one thing: over time, I've really come to regret the "implicit" FileStore creation that I originally implemented way before filestore was split out of Dmedia. For example, say you have an empty directory /tmp/foo, currently this works:

>>> fs = FileStore('/tmp/foo')

And it will implicitly create the /tmp/foo/.dmedia directory and the FileStore layout within. This turned out the be a bad idea. It's far better for code to either 1) assume the FileStore already exists, or 2) assume the FileStore needs to be created. But don't mix the two.

Err, I lied... there is actually another thing, closely related. Overall, I tried to keep filestore and Dmedia cleanly separated, and for filestore to be unaware of Dmedia schema decisions. So previously I had Dmedia create the dmedia/store doc in CouchDB, and then to provide the _id and copies to the FileStore constructor:

FileStore.__init__(parentdir, _id=None, copies=0)

But this doesn't work well in a world where a FileStore is never implicitly created. It's much cleaner and simpler to have the presence of the .dmedia/store.json file indicate whether the FileStore was completed created. But that means moving the doc creation into filestore (although keeping it unaware of the database and other schema details). So I'm proposing the new FileStore constructor be:

FileStore.__init__(parentdir, expected_id=None)

The above will never implicitly create a FileStore that doesn't already exist. And the new `expected_id` kwarg is for subprocesses that create FileStore instances... this is to make sure the doc['_id'] of the FileStore they load matches what the parent process provided them.

And there will also be a new FileStore.create() classmethod:

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

It will fail if a FileStore already exists in *parentdir*, and will otherwise return a FileStore instance.

Related branches

Changed in filestore:
milestone: 13.04 → 13.05
Jason Gerard DeRose (jderose) wrote :

Note that as it was much easier to first do the V0 => V1 switch over, and then make the FileStore creation always be explicit, that's what I'm doing.

Current work is now happening in lp:~jderose/filestore/explicit-create

lp:~jderose/filestore/migration is being left for reference, but was a dead-end road.

summary: - Make V1 protocol default, remove implicit FileStore creation
+ Require explicit FileStore creation
Changed in filestore:
status: In Progress → Fix Committed
Changed in filestore:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers