odoo-extra icon indicating copy to clipboard operation
odoo-extra copied to clipboard

Add debug_server_actions module

Open jrial opened this issue 10 years ago • 4 comments

Added functionality for debugging server actions, but this time as a module rather than updating the Odoo core.

jrial avatar Sep 12 '14 12:09 jrial

In fact it is worse in a external module as you duplicate the whole code of safe_eval (it's already out of sync)

maybe, only monkey-patching test_expr (same signature) and reading the debug_fd from a threadlocal

Then, the function safe_eval_debug look like

def safe_eval_debug(...):
    debug = local.debug = debug and  config.get('debug_mode')
    local.fd = None
    if not debug:
        return safe_eval(...)
    with tempfile.NamedTemporaryFile(suffix='.py') as debug_fd:
        # store `debug_fd` to the threadlocal
        # FIXME use a weakref? or just the fd (int) and use os.open() in test_expr?
        local.fd = debug_fd
        return safe_eval(...)

KangOl avatar Oct 03 '14 12:10 KangOl

No need to preach to the choir; I made the same argument about code duplication when the initial patch was rejected. This current solution was seen as a compromise between not wanting to alter too much code in the core of OpenERP for something that's rarely used, while still providing it for those who need it.

Been looking around a bit at your proposal, but unless I'm missing something, it wouldn't work. I can't just write a wrapper around safe_eval and call safe_eval in its original form, optionally setting up some thread-local storage for test_expr. The safe_eval_debug method does not just pass the debug argument to test_expr_debug, but also calls either eval or pdb.runeval, depending on whether you're debugging or not.

jrial avatar Oct 15 '14 14:10 jrial

Why is the call to pdb.runeval needed?

KangOl avatar Oct 15 '14 15:10 KangOl

Ugh, sorry for the slow reply. Was a bit caught up in other work.

Reason I do a runeval is to prevent people from shooting themselves in the foot. I could also have done it so that including PDB was permissible, or automatically done, so they could just insert pdb.set_trace() wherever they want in the SA code. But someone, somewhere is bound to forget to remove it when copying the SA to production.

So in my case, the code for the SA does not need to be modified for debugging, and the debugger initiates right at the beginning of the SA. Nothing you do to the SA will cause it to work perfectly on an environment configured for debugging, while failing/stacktracing in production.

jrial avatar Nov 04 '14 16:11 jrial