Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pre "chaining" appears to be broken #120

Closed
sigil66 opened this issue Feb 27, 2014 · 4 comments
Closed

pre "chaining" appears to be broken #120

sigil66 opened this issue Feb 27, 2014 · 4 comments

Comments

@sigil66
Copy link

sigil66 commented Feb 27, 2014

When utilizing the following code via collection 'test' precheck() never executes. If all the methods are specified in pre via default= then the expected behavior is observed.

   from invoke import task

    @task
    def precheck():
        print("Preing")

    @task(pre=['test.precheck'])
    def check():
        print("Checking")

    @task(default=True, pre=['test.check'])
    def run():
        print("Defaulting")
@bitprophet
Copy link
Member

Related to #123 though a bit different.

Not sure this angle has been tested/thought about either; I would assert that you don't want the tasks to have to know "which" collection they're in at the moment, in case that changes/gets renamed/etc.

Meaning the above should turn into eg @task(pre='precheck') (n.b. you can just skip 'pre' and say @task('precheck')) and we should ensure the code correctly handles this (I'm not sure it would at the moment.)

@sigil66
Copy link
Author

sigil66 commented Mar 4, 2014

Well I tried that and it errored out with a task not found error when the task was not namespaced.

@bitprophet
Copy link
Member

At this point in time the string version of pre-task declaration is dead, so that removes that part of the confusion.

While redoing the pre-task stuff, there's definitely situations that are broken which resemble the above (e.g. a pretask of a pretask is not executed). Will be ensuring that this case works before the new functionality is released. (See also #123)

@bitprophet
Copy link
Member

Pre-emptively closing this now that the work is basically all done. Using #123 as the place to keep final notes as I wrap it up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants