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

Feature request: better logging for nested tasks #24

Open
ghost opened this issue Apr 11, 2020 · 9 comments
Open

Feature request: better logging for nested tasks #24

ghost opened this issue Apr 11, 2020 · 9 comments

Comments

@ghost
Copy link

ghost commented Apr 11, 2020

Use case: I have a main task that performs actions on items of an iterable, so instead of specifying the one-item task as dependency, I call it directly. Here's my code:

#!/usr/bin/env python3

from pynt import task

ITEMS = ["foo", "bar"]


@task()
def item_task(item):
    print(item)


@task()
def main_task():
    for item in ITEMS:
        item_task(item)


__DEFAULT__ = main_task

This is the output I'm getting:

[ build.py - Starting task "main_task" ]
foo
bar
[ build.py - Completed task "main_task" ]

Since item_task() is a task too, I'd prefer the following output:

[ build.py - Starting task "main_task" ]
[ build.py - Starting task "item_task" with argument "foo"]
foo
[ build.py - Completed task "item_task" ]
[ build.py - Starting task "item_task" with argument "bar"]
bar
[ build.py - Completed task "item_task" ]
[ build.py - Completed task "main_task" ]

Maybe even with nested tasks indented:

[ build.py - Starting task "main_task" ]
    [ build.py - Starting task "item_task" with argument "foo"]
    foo
    [ build.py - Completed task "item_task" ]
    [ build.py - Starting task "item_task" with argument "bar"]
    bar
    [ build.py - Completed task "item_task" ]
[ build.py - Completed task "main_task" ]
@ghost
Copy link
Author

ghost commented Apr 12, 2020

I was able to get a PoC first option (without indenting nested tasks) by patching Task.__call__(), but I don't like this ugly hack:

import pynt

def custom_call(self, *args, **kwargs):
    local_args = locals()
    task_name = vars(local_args["self"])["name"]
    del local_args["self"]
    print("[ Starting task \"{}\" with arguments {}]".format(task_name, local_args))
    self.func.__call__(*args, **kwargs)
    print("[ Completed task \"{}\" ]".format(task_name))

pynt._pynt.Task.__call__ = custom_call

from pynt import task

@rags
Copy link
Owner

rags commented Apr 13, 2020 via email

@ghost
Copy link
Author

ghost commented Apr 13, 2020

Other option is to have custom logging inside the task

I've been thinking to play a bit with the code and go this way, but after we reach some consensus about broken tests (see #25), since I'd like to be able to run tests while working on code, to make sure I'm not breaking anything.

@ghost
Copy link
Author

ghost commented Apr 14, 2020

I have tried to move

logger.info("Starting task \"%s\"" % task.name)
and
logger.info("Completed task \"%s\"" % task.name)
into
def __call__(self,*args,**kwargs):

But I've got stuck because I don't know how to pass the logger instance into Task :(

@ghost
Copy link
Author

ghost commented Apr 14, 2020

Please see https://github.com/somospocos/pynt/commit/856557af703b41436da48bf7b665f2eb2084c06c

I was able to avoid passing the logger instance around between functions by using a global. I know using globals is generally not recommended, but I simply don't know how to do it better for now. If you have any suggestions how to improve this, let me know. If you're willing to accept this into upstream, let me know as well, so I'll make a pull request.

@rags
Copy link
Owner

rags commented Apr 16, 2020 via email

@ghost
Copy link
Author

ghost commented Apr 16, 2020

I am a bit confused because you seem to address #25 here. Do you want me to modify build.py locally, so tests run in my setup, but prefer to keep build.py in your repository as it is? Please answer in issue 25 regarding this, so we could keep the things isolated from eachother.

As for this issue, which is about better logging, I have something else to show. I have improved logging a bit further, on top of previous change, so it can show task arguments if required https://github.com/somospocos/pynt/commit/807f4503113ecea4006cedcffe9fd34457f92f69

So my plan currently is:

  1. wait for your feedback regarding tests failing on my setup (again, please answer that in issue 25)

  2. if you prefer to keep build.py as it is, I'm fine with that and can run the tests locally with a modified version of build.py

  3. I will need your feedback here about the two features I've added:

    a. logging for nested tasks
    b. verbose logging that displays arguments tasks were called with

    I'd like to know if you want one of them, both of them, and if you want both - should they be formatted as single pull request or as separate ones.

@rags
Copy link
Owner

rags commented Apr 17, 2020 via email

@ghost
Copy link
Author

ghost commented Apr 17, 2020

I am using the unmodified build.py from the first post in this issue and the private branch https://github.com/somospocos/pynt/tree/private from my fork

This is the output I'm getting:

$ pynt
[ build.py - Starting task "main_task" ]
[ build.py - Starting task "item_task" ]
foo
[ build.py - Completed task "item_task" ]
[ build.py - Starting task "item_task" ]
bar
[ build.py - Completed task "item_task" ]
[ build.py - Completed task "main_task" ]

$ pynt -V                                                                                 
[ build.py - Starting task "main_task" ]
[ build.py - Starting task "item_task" with arguments ['foo'] ]
foo
[ build.py - Completed task "item_task" ]
[ build.py - Starting task "item_task" with arguments ['bar'] ]
bar
[ build.py - Completed task "item_task" ]
[ build.py - Completed task "main_task" ]

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

1 participant