fab-user
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Travis Swicegood
Subject: Re: [Fab-user] Code review for changes in 1.x
Date: Mon, 14 Feb 2011 11:05:27 -0600

On Mon, Feb 14, 2011 at 10:36 AM, Michael Gliwinski <address@hidden> wrote:
> > 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...

We could -- that was against the idea behind this style.  I wanted to get something functioning first, then we're refactor under the hood.  A @task is a @task is a @task as far as the fabfile user/writer is concerned.  How it's stored internally is something beyond their concern. Now, the namespaced tasks are a different story.

 
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".

We could.  I thought about going this route and only scanning if tasks had been registered.  But what happens when you import a third-party module that uses @task and you don't?  Now there's things in the registry, so we can't know for sure if you've registered all of your tasks or not.

 
> > 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.

I would be open to starting the registry refactor and using a fabric.api.register_as_task_module() that would work the same as the variable does.  Inside the vendor.tasks.apache, for example, instead of FABRIC_TASK_MODULE, they would import register_as_task_module() and call that.  Then the scanning code could check variables against a list of registered modules rather than a simple variable check.

I did contemplate going that route, but landed on the variable simply because it was the simplest solution.

 
>  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).

There are two use cases.  The one you mentioned (fab production.deploy, fab staging.deploy).  I use that quite a bit.

The other case that I'm hoping this encourages is shared stuff.  For example fab apache.restart, fab django.init, and so on.  This would make it possible for people to package common tasks and distribute them that way.  I know of several developers (myself included) who started down this path only to discover we had to use PEAR-style prefixing for everything.

 
> 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)?

And this would exist in the fabfile?

-T

reply via email to

[Prev in Thread] Current Thread [Next in Thread]