Re: [Fab-user] Code review for changes in 1.x

[Michael Gliwinski]

On Monday 14 Feb 2011 14:31:32 Travis Swicegood wrote:
> Agreed -- this started out as a simple fix or two, then ballooned.  There
> are a few pieces to this that can be extracted, but for the most part the
> __all__, namespace, and @task code depends on refactoring done along the
> way so extracting it would be rather hard.

Yeah, I actually only meant the smaller independent changes.  Namespace 
support and task designation do fit together anyaways, IMO.

> > Regarding overall approach, what made you choose the Task class
>
> The reason to move to the Task was to get a nice flex point that allows
> other developers to provide custom types of tasks.  Think @depends that
> returns a class that extends Task (or WrappedCallableTask) and has a custom
> run method for checking that other tasks have run.
> 
> The second reason has not yet started -- moving the actual task
> setup/execution out of fabric.main into Task.run.  Currently if I call
> another task function inside a task it executes the function, which may or
> may not be what Fabric does.

OK, yeah, that makes sense regardless if tasks are stored in some sort of 
registry or not.

> > By registry here I mean either a simple dict as is already used or
> > something
> > more fancy like e.g. Registry implementation in bzrlib [1] which allows
> > storing both objects (or indirect references) and metadata.
> 
> I agree.  I think the the registry needs to be expanded a little bit.
>  Providing a registry.find_task(name='foo') and the ability to check to see
> if a task has run (and how many times) are something that definitely need
> to happen.  My original thought for @task was that it would register tasks
> directly with a registry.  Given the current implementation, however, I
> opted for the scanning of a variable to see if it was a task since it is
> currently a registry of "sorts". :-)

Hmm, I see your point, however I'm wondering if it might be better then to 
formalize and clean up that registry first...

Fair enough, after a second look it seems to me a lot of that loading/scanning 
code would stay mostly as is to preserve BC even if we did have a nice formal 
"registry of tasks".

> > I like the namespacing implementation, although because "explicit is
> > better
> > than implicit" I agree with Jeff's comment to #56 [2] about having a
> > register_subtasks or similar function.
> 
> Let me think on this one.  My initial inclination leans slightly toward
> putting it on the task designer to specify what can be used as a task and
> what can't, but I can definitely see the point of it feeling a bit magic.

I agree!

Hmm, I'll think about it some more too.  I think the designer should specify 
where the tasks are but something doesn't feel right about having to 
explicitly trigger scanning a submodule.

>  My reasoning for putting it in the module itself, rather than the fabfile
> is that the module gets written once, the fabfiles get written multiple
> times.  It seems reasonable to write it once, rather than having to
> explicitly register it each time.

Ah, OK, just noticed we are probably thinking about different use cases.  I 
see namespaces as just a neat way to partition a fabfile into components (I 
have some huge fabfiles which could use that :))  I.e. parts of one fabfile 
package related to a particular system/project/domain.

For shared functionality between different fabfiles I just use a separate 
Python package (here's where setuptools-like extending of fabric namespace 
would be useful though).

> One point, the register_subtasks code suffers from the same problem that
> lead to scanning for @task's rather than having @task actually register --
> we currently scan the file for tasks rather than having an explicit
> registry to stuff things into.  Now, it could be that we import the

Hmm, actually having to scan submodules one way or another is the problem.  
Still we have to do that for BC.  Maybe just being able to limit where to 
scan?  (aka __all__ for submodules to include)?


-- 
Michael Gliwinski
Henderson Group Information Services
9-11 Hightown Avenue, Newtownabby, BT36 4RT
Phone: 028 9034 3319

**********************************************************************************************
The information in this email is confidential and may be legally privileged.  
It is intended solely for the addressee and access to the email by anyone else 
is unauthorised.
If you are not the intended recipient, any disclosure, copying, distribution or 
any action taken or omitted to be taken in reliance on it, is prohibited and 
may be unlawful.
When addressed to our clients, any opinions or advice contained in this e-mail 
are subject to the terms and conditions expressed  in the governing client 
engagement leter or contract.
If you have received this email in error please notify address@hidden

John Henderson (Holdings) Ltd
Registered office: 9 Hightown Avenue, Mallusk, County Antrim, Northern Ireland, 
BT36 4RT.
Registered in Northern Ireland
Registration Number NI010588
Vat No.: 814 6399 12
*********************************************************************************