ionelmc / python-hunter

Hunter is a flexible code tracing toolkit.
https://python-hunter.readthedocs.io/
BSD 2-Clause "Simplified" License
793 stars 45 forks source link

Conditional breakpoints, object attribute mutation detection and hunter #57

Open saaj opened 5 years ago

saaj commented 5 years ago

In the sense of this StackOverflow answer do you think the following predicate object is worth inclusion into the library:

import traceback

class MutationWatcher:

    def __init__(self, target, attrs):
        self.target = target
        self.state = {k: getattr(target, k) for k in attrs}

    def __call__(self, event):
        result = False
        for k, v in self.state.items():
            current_value = getattr(self.target, k)
            if v != current_value:
                result = True
                self.state[k] = current_value
                print('Value of attribute {} has chaned from {!r} to {!r}'.format(
                    k, v, current_value))

        if result:
            traceback.print_stack(event.frame)

        return result

If so, does it need any improvements (apart from a docstring)? Better name?

ionelmc commented 5 years ago

At the very least this is a good cookbook example (https://github.com/ionelmc/python-hunter/blob/master/docs/cookbook.rst).

For having something for this builtin I think it should be easy to use from environment. So it needs some import helpers at least. Eg, this should work: PYTHONHUNTER=Watch('pkg.mod.inst.attr') as well as this:

from pkg.mod import inst
with hunter.trace(hunter.Watch(inst, 'attr')): 
   ...

Also there's the question of there to put this, it's not usable as an action (eg: module='foobar',action=Watch(...) will not work properly) but it has output like an action. Hmmmm.

saaj commented 5 years ago

I used it as a predicate. The action in my case (debugging SQLAlchemy intricacies) is either code printer or debugger. The predicate though prints stuff on its own, which is odd.

To be usable via PYTHONHUNTER it needs to be more robust. Namely, account for runtime defined attributes, properties that raise, etc (and good to have anyway). But I have some doubts on practicality of this way of using it, because objects of interest usually appear somewhere in between the call stack and are not accessible like pkg.mod.inst.attr. That said, inspecting locals of the frame could be more useful.

ionelmc commented 5 years ago

For sure passing it the object would be more flexible, I'm only asking that it should be usable with a string target (for ease of use from env, with simpler/limited targets of course).

So it would need to inherit BaseAction for having output consistent with the other actions. Note that predicates come in 2 implementations (cython/pure).

You interested in making a PR for this? (I can do the cython implementation if it's too much)

saaj commented 5 years ago

I'm interested.

ionelmc commented 5 years ago

Another thing, I just noticed you mentioned "conditional breakpoints".

So for Watch(foo, 'bar'), action=Debugger to work we'd need to make Watch's __call__ return True every time foo.bar changed, and False otherwise.

However that makes it unusable if you only want it to print the value when it has changed. In other words it would work as an event filter, people might want to see what code runs in addition to watching the attribute. The situation could be solved by two ways I think:

Also there's some overlap with VarsPrinter - not really a fan of overlapping functionality - I would like to have things orthogonal. Lemme think a bit about this ...

ionelmc commented 5 years ago

So I put these examples in two colums:

VarsWatcher("mod.obj.attr != 0")                          VarsWatcher("mod.obj[0] != 0") 
VarsWatcher("mod.obj.attr")                               VarsWatcher("mod.obj[0]")        
VarsWatcher("len(mod.obj.attr) != 0")                     VarsWatcher("len(mod.obj[0]) != 0") 
VarsWatcher("len(mod.obj.attr)")                          VarsWatcher("len(mod.obj[0])")        

VarsWatcher("obj.attr != 0",      locals={'obj': obj})    VarsWatcher("obj[0] != 0",      locals={'obj': obj}) 
VarsWatcher("obj.attr",           locals={'obj': obj})    VarsWatcher("obj[0]",           locals={'obj': obj})        
VarsWatcher("len(obj.attr) != 0", locals={'obj': obj})    VarsWatcher("len(obj[0]) != 0", locals={'obj': obj}) 
VarsWatcher("len(obj.attr)",      locals={'obj': obj})    VarsWatcher("len(obj[0])",      locals={'obj': obj})        

Your MutationWatcher(obj, 'attr') is equivalent to VarsWatcher("obj.attr", locals={'obj': obj}).

The actual changes I'm proposing:

Now I realize this is way more than you bargained for, and may be overkill for your really simple use case. And it's less efficient than your simple MutationWatcher. The reasoning is that it should be more general so users don't have to copy-paste the class and change some stuff just to support watching the length of a list or looking up an item.

I would also like to discuss some alternatives. Eg: a proposal for a plugin system so you can pip install hunter-my-simple-predicates or pip install hunter-saaj and extra objects would be available in the evaluation context for PYTHONHUNTER and maybe even in the hunter module.

What do you think?

ionelmc commented 5 years ago

@saaj So I've been thinking a bit more about this, and while I don't like the overlap with VarsPrinter now I think your initial proposal is way more simpler, and better than the ideas I thrown around.

You still interested in adding a PR with this API?

MutationWatcher(obj, 'attr')
MutationWatcher('pkg.mod.obj.attr')