Improved context/collection configuration #107

Closed
bitprophet opened this Issue Jan 10, 2014 · 5 comments

Projects

None yet

1 participant

@bitprophet
Member

Currently configuration is 'global' - everything gets merged into a single config dict, with inner/outer collections collapsing, and - specifically - 'siblings' getting walked in a specific order, etc.

This works great until you want to import two 'copies' of a collection into the same level of your namespace, and parameterize them differently (in my case, I now have a project w/ two different Sphinx docroots.)

Feels like a good way to support this while not killing the 'old' use case (wherein an importing collection can override the imported one) is to remove sibling walking and also be 'smarter', looking 'top down' towards the specific collection the invoking task is in.

In other words, this:

from invoke import Collection, task
from invocations import docs

api = Collection.from_module(docs)
site = Collection.from_module(docs)
api.configure({'sphinx.source': 'api'})
site.configure({'sphinx.source': 'site'})

@task
def somelocaltask():
    pass

ns = Collection(somelocaltask, api=api, site=site)

doesn't work right now, because the "give me a single global config" algorithm walks the two siblings 'api' and 'site' and one of them wins over the other.

@bitprophet
Member

Specifically this means that Collection.configuration can no longer be a naive dict but needs to be a method parameterized such that the value given informs which 'path' to take up the tree (i.e. the task, or just cut to the chase and give it the 'leaf' subcollection itself.)

This needs to work for any number of levels, so probably just recursive calls and have it return None or some sentinel value when it can't "find" the desired leaf.

@bitprophet
Member

Found a wrinkle - what if a collection was copied around multiple times? In the above scenario the two "copies" of 'docs' are distinct Collection objects - but somebody might want to legitimately make a given collection available, unparameterizedm in 2 different paths.

Makes me realize we should just be going by the literal invocation, e.g. inv foo.bar.baz -> merge 'baz' into 'bar', then the result into 'foo', then the result into the root. No 'crawling' necessary and allows Collection.configuration to remain a simple dict (in fact, it doesn't even need to do recursion at all any longer - merging is now the province of Executor entirely.)

Herp and a derp.

EDIT: Well, yes and no, I can still do it via Collection, and should, because Collection already handles task lookup via the dotted notation stuff, recursively. So config should follow that, otherwise it feels janky and inconsistent.

@bitprophet
Member

Got tests passing and all on first try

Unfortunately...real world setup still sees both copies getting the same settings :(

Turns out that they are the same object so it's literally getting overwritten. Meaning Collection.from_module is returning the same object both times, somehow. Looking into that.

@bitprophet
Member

Hah, took me 3 commits worth of new tests to find it - was specifically the 'root ns' object that did it, from_module was returning the object unmodified instead of wrapping in a new collection, and my earlier tests against this issue were loading a support module that didn't have a root NS.

Seems to work now \o/

@bitprophet bitprophet added a commit that referenced this issue Jan 13, 2014
@bitprophet bitprophet DDD re #107 acc2bb5
@bitprophet bitprophet added a commit that referenced this issue Jan 13, 2014
@bitprophet bitprophet Changelog re #107 a4f0a7d
@bitprophet
Member

All done.

@bitprophet bitprophet closed this Jan 13, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment