pynt icon indicating copy to clipboard operation
pynt copied to clipboard

Feature request: better logging for nested tasks

Open ghost opened this issue 4 years ago • 9 comments

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 avatar Apr 11 '20 13:04 ghost

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

ghost avatar Apr 12 '20 23:04 ghost

One option is to use the dependency chain instead of calling the task like a function. But you may need separate tasks for each param in this case. Other option is to have custom logging inside the task

On Mon, Apr 13, 2020 at 1:21 AM somospocos [email protected] wrote:

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

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/rags/pynt/issues/24#issuecomment-612691067, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABCJUSY63U7YEGLAP3DN2TRMJEHFANCNFSM4MF7XTXQ .

rags avatar Apr 13 '20 19:04 rags

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 https://github.com/rags/pynt/issues/25), since I'd like to be able to run tests while working on code, to make sure I'm not breaking anything.

ghost avatar Apr 13 '20 19:04 ghost

I have tried to move https://github.com/rags/pynt/blob/c0fbbc4c0c6c1ccbbf11a744fb62d33c78ac0ae0/pynt/_pynt.py#L188 and https://github.com/rags/pynt/blob/c0fbbc4c0c6c1ccbbf11a744fb62d33c78ac0ae0/pynt/_pynt.py#L198 into https://github.com/rags/pynt/blob/c0fbbc4c0c6c1ccbbf11a744fb62d33c78ac0ae0/pynt/_pynt.py#L256

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

ghost avatar Apr 14 '20 10:04 ghost

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.

ghost avatar Apr 14 '20 23:04 ghost

Hi, I think the Travis is able to run 2.7 and 3.3 tests on github. So if you can locally change the build file to just run with 3.x you have. That should be sufficient. Your patches will be validated by automated build once you are ready

On Mon, Apr 13, 2020 at 9:21 PM somospocos [email protected] wrote:

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 https://github.com/rags/pynt/issues/25), since I'd like to be able to run tests while working on code, to make sure I'm not breaking anything.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rags/pynt/issues/24#issuecomment-613052701, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABCJUSKGQ5ZAVIXQQ4DA5LRMNQ4VANCNFSM4MF7XTXQ .

rags avatar Apr 16 '20 19:04 rags

I am a bit confused because you seem to address https://github.com/rags/pynt/issues/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.

ghost avatar Apr 16 '20 19:04 ghost

My misunderstood that you were blocked because of the tests. From your poc code, I dont see how it will work. call is called if pynt resolves it as a task and explicitly calls the created task. In your usecase you are making a function call and not a task call.

I think another work around would be to expose get_task_by_name(name) [This doesnt exist in this form] in pynt and do something like this

from pynt import get_task_by_name

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

@task() def main_task(): for item in ITEMS: pynt.get_task_by_name('item_task')(item)

On Thu, Apr 16, 2020 at 9:43 PM somospocos [email protected] wrote:

I am a bit confused because you seem to address #25 https://github.com/rags/pynt/issues/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 somospocos@807f450 https://github.com/somospocos/pynt/commit/807f4503113ecea4006cedcffe9fd34457f92f69

So my plan currently is:

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.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rags/pynt/issues/24#issuecomment-614857802, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABCJUVUHYZ7EB7XH5QB2GLRM5NXDANCNFSM4MF7XTXQ .

rags avatar Apr 17 '20 09:04 rags

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

ghost avatar Apr 17 '20 11:04 ghost