Re: [Fab-user] Code review for changes in 1.x
Sorry for late reply, was away from my email most of the day yesterday. On Tuesday 15 Feb 2011 13:39:49 Travis Swicegood wrote: > On Tue, Feb 15, 2011 at 6:47 AM, Michael Gliwinski < > > michael.gliwin...@henderson-group.com> wrote: > > Wouldn't shared functionality rather be something that you use in tasks > > in your fabfile rather than exposing it as tasks on command line? I > > mean, yeah > > there are few small and simple bits like .restart or shell > > or even wrappers for some remote commands you may run interactively like > > grep, etc. but most of shared stuff I have is utilities/objects I only > > use in actual fabfile tasks. > > > > ... snip ... > > > > Do you have lot of shared functionality which is exposed directly? > > It's mixed. I have a lot of tasks that are completely useful on their own > or composed as part of a larger package. For example, my rabbitmq tasks[1] > are normally combined into a rabbitmq_init task that sets it up the way I > expect, but it's also nice to be able to interact directly with rabbitmq as > needed. Hmm, good point. > > Look at it this way: modules/packages with shared functionality have to > > be installed globally (specifically somewhere on Python's path so it's > > accessible > > from different fabfiles), now for example you have a module 'apache' > > which provides ways to start, stop, configure, check status of the > > server, etc.; now > > you have a fabfile which is meant to be used by support group for > > diagnostics > > of various systems including an apache server; if you import that shared > > apache module to get the status command all of a sudden you also get one > > to reconfigure the server. > > You could always import simply the status function from the apache module. > All this code does is scan the fabfile looking for potential modules. If > you don't have an `apache` variable, it's not going to know there's > anything there to scan. Could do now, but not if tasks are self-registering. Suppose at this point I'm actually talking more kind of "ideal to eventually have" design for this feature so it doesn't end up complicated and inconsistent. Anyway, I'm not saying I'd want every fabfile to have long list of "expose this and that imported task" but that eventually even such level of granularity may be useful as an option. In most cases I'd expect "expose everything" as a default and some form of limiting by module to work OK. > All that said, what you're describing here is a selective import of tasks > that retain their module relationship. I love that idea, but I think it's > a next version of this. The first version should be dead simple (either a > variable signifying task modules or some Fabric API for signifying task > modules) and we can start doing things like filtering as a second version. Indeed, don't get me wrong, I agree that functionality (namespaces and explicit tasks) is needed and can be extended later. In any case, I'll try to test the code more throughly in the coming week and provide some feedback and in the meantime we can see what Jeff thinks about this. -- 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 supp...@henderson-group.com 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 * ___ Fab-user mailing list Fab-user@nongnu.org http://lists.nongnu.org/mailman/listinfo/fab-user
Re: [Fab-user] Code review for changes in 1.x
On Tue, Feb 15, 2011 at 6:47 AM, Michael Gliwinski < michael.gliwin...@henderson-group.com> wrote: > > > 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. > > Wouldn't shared functionality rather be something that you use in tasks in > your fabfile rather than exposing it as tasks on command line? I mean, > yeah > there are few small and simple bits like .restart or shell or > even wrappers for some remote commands you may run interactively like grep, > etc. but most of shared stuff I have is utilities/objects I only use in > actual fabfile tasks. > > ... snip ... > > Do you have lot of shared functionality which is exposed directly? > It's mixed. I have a lot of tasks that are completely useful on their own or composed as part of a larger package. For example, my rabbitmq tasks[1] are normally combined into a rabbitmq_init task that sets it up the way I expect, but it's also nice to be able to interact directly with rabbitmq as needed. > Look at it this way: modules/packages with shared functionality have to be > installed globally (specifically somewhere on Python's path so it's > accessible > from different fabfiles), now for example you have a module 'apache' which > provides ways to start, stop, configure, check status of the server, etc.; > now > you have a fabfile which is meant to be used by support group for > diagnostics > of various systems including an apache server; if you import that shared > apache module to get the status command all of a sudden you also get one to > reconfigure the server. You could always import simply the status function from the apache module. All this code does is scan the fabfile looking for potential modules. If you don't have an `apache` variable, it's not going to know there's anything there to scan. All that said, what you're describing here is a selective import of tasks that retain their module relationship. I love that idea, but I think it's a next version of this. The first version should be dead simple (either a variable signifying task modules or some Fabric API for signifying task modules) and we can start doing things like filtering as a second version. -T [1]: https://github.com/domain51/d51.fabric.tasks.rabbitmq ___ Fab-user mailing list Fab-user@nongnu.org http://lists.nongnu.org/mailman/listinfo/fab-user
Re: [Fab-user] Code review for changes in 1.x
On Monday 14 Feb 2011 17:05:27 Travis Swicegood wrote: > michael.gliwin...@henderson-group.com> wrote: > > 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. Yeah, what I meant below is that indeed it's good as it is for now as it preserves BC, etc. Good point about it being hidden so an under the hood refactor should be easy later. > > 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. OK, I see your point. > > > > 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. I was thinking of namespaces as means of splitting fabfiles in this case. See below for my general reasoning on shared functionality. > > > 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. Wouldn't shared functionality rather be something that you use in tasks in your fabfile rather than exposing it as tasks on command line? I mean, yeah there are few small and simple bits like .restart or shell or even wrappers for some remote commands you may run interactively like grep, etc. but most of shared stuff I have is utilities/objects I only use in actual fabfile tasks. For example: I have a shared RPMBuilder class which wraps mock on a build server to build RPMs, I do not expose it directly on fab command line but instead in each project's fabfile I have a command/task which calls this RPMBuilder with options specific for this project. In those cases where a shared function makes sense to be used directly as fab command I think I'd rather want to be able to control the fact that it's exposed from the particular fabfile being used. Do you have lot of shared functionality which is exposed directly? > > > One point, the register_subtasks code suffers from the same problem > > > that lead to scanning for @task's rather than having
Re: [Fab-user] Code review for changes in 1.x
On Mon, Feb 14, 2011 at 10:36 AM, Michael Gliwinski < michael.gliwin...@henderson-group.com> 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 ___ Fab-user mailing list Fab-user@nongnu.org http://lists.nongnu.org/mailman/listinfo/fab-user
Re: [Fab-user] Code review for changes in 1.x
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 supp...@henderson-group.com John Henderson (Holdings) Ltd Registered office: 9 Hightown Avenue, Mallusk, County Antrim, Northern Ireland, BT36 4RT. Registere
Re: [Fab-user] Code review for changes in 1.x
On Mon, Feb 14, 2011 at 7:46 AM, Michael Gliwinski < michael.gliwin...@henderson-group.com> wrote: > On a general note though, wouldn't it be easier (for reviewers and for > merging) if these things were separated into feature branches? The > different > features there don't seem to depend on each other anyways. > 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. Regarding overall approach, what made you choose the Task class as a way to > designate tasks? I'm asking because it doesn't appear to do anything > special > and has a disadvantage of replacing the original callable and changing its > signature. > 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. Finally, classes provide a nice hook to find out whether a possible task is using Task. Since we want this to be BC, the idea is to only switch to @task decorators if one is present. All of this could be done via straight decorators and variables set on those decorators. It's 6 of one to a half dozen of another. > I'm just thinking aloud here but it seems to me a registry approach may be > simplier. Fabric already has a sort of task registry (the tasks dict used > in > load_fabfile) so it would be a matter of making it global and changing how > it's populated. > > 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". :-) 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. 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. 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 fabfile, have @task / register_module_tasks mark their variables as already_processed or some such, but that's a slightly bigger change than what's in the code right now. -T ___ Fab-user mailing list Fab-user@nongnu.org http://lists.nongnu.org/mailman/listinfo/fab-user
Re: [Fab-user] Code review for changes in 1.x
On Sunday 13 Feb 2011 16:50:10 Travis Swicegood wrote: > As many of you may know, I've long had a fork of Fabric that adds some > goodies in such as @task decorators and module level tasks (i.e., fab > prod.deploy) and a few other things. I've created a Pull Request on GitHub Good job! There are some nice things in there that I'd be happy to see merged into Fabric indeed. On a general note though, wouldn't it be easier (for reviewers and for merging) if these things were separated into feature branches? The different features there don't seem to depend on each other anyways. > Any thoughts you have would be appreciated. I don't think this will be the > final solution for @task style task declaration, and I know it isn't the > final Task object. There's still a lot to add/refactor before this is > through. The changes in this pull request are meant as a first step, and > as such are entirely backwards compatible. Regarding overall approach, what made you choose the Task class as a way to designate tasks? I'm asking because it doesn't appear to do anything special and has a disadvantage of replacing the original callable and changing its signature. I'm just thinking aloud here but it seems to me a registry approach may be simplier. Fabric already has a sort of task registry (the tasks dict used in load_fabfile) so it would be a matter of making it global and changing how it's populated. 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 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. [1]: http://bazaar.launchpad.net/~bzr- pqm/bzr/bzr.dev/view/head:/bzrlib/registry.py#L74 [2]: http://code.fabfile.org/issues/show/56#note-3 -- 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 supp...@henderson-group.com 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 * ___ Fab-user mailing list Fab-user@nongnu.org http://lists.nongnu.org/mailman/listinfo/fab-user
