Getting a proper return code from luigi

[Noah Yetter]

The docs now recommend to run Luigi programs thusly:

$ luigi --module my_module EntryTaskClass --your --arguments --here

This doesn't work for me for a variety of reasons, so I cracked the luigi "binary" open to see what it does:

#!/path/to/my/bin/python

# -*- coding: utf-8 -*-

import re

import sys

from luigi.cmdline import luigi_run

if __name__ == '__main__':

    sys.argv[0] = re.sub(r'(-script\.pyw?|\.exe)?$', '', sys.argv[0])

    sys.exit(luigi_run())

OK, simple enough. I'll just import luigi_run in my code and run it:

from luigi.cmdline import luigi_run

# rest of my code snipped...

if __name__ == '__main__':

    luigi_exit_code = luigi_run()

    LOG.info('luigi exit code: {}'.format(luigi_exit_code))

    # do some other cleanup...

    sys.exit(luigi_exit_code)

It works, but it doesn't. Luigi runs all my tasks, but my log statement never happens, nor does my post-run cleanup. So I looked further into what luigi_run does:

# excerpt from luigi/cmdline.py

from luigi.retcodes import run_with_retcodes

def luigi_run(argv=sys.argv[1:]):

    run_with_retcodes(argv)

Ooooookaaaay, that makes no sense. The luigi_run function does not return anything, so why is it called as sys.exit(luigi_run()) in the luigi script? Let's go deeper and have a look at run_with_retcodes...

# excerpt from luigi/retcodes.py

def run_with_retcodes(argv):

    # snip snip snip

    if expected_ret_code == 0 and \

       root_task not in task_sets["completed"] and \

       root_task not in task_sets["already_done"]:

        sys.exit(retcodes.not_run)

    else:

        sys.exit(expected_ret_code)

Ugh, facepalm. Look at those sys.exit() calls buried deep in this function! A caller has no hope of running cleanup code after invoking luigi_run() with the code structured this way, without using an atexit handler (which can't see the value passed to sys.exit) or monkey-patching sys.exit (which is icky).

For giggles, let's compare this to the "old" recommended way of running Luigi programs, using luigi.run() (run is imported from luigi.interface)...

# excerpt from luigi/interface.py

def run(*args, **kwargs):

    return _run(*args, **kwargs)['success']

Ah ha, a proper return! Unfortunately it's a boolean, so we lose the fine-grained (and configurable!) exit status provided by retcodes. And in practice, at least with the configuration recommended by the docs, a successful run that had a retry returns False, so it looks like a failure to the shell, which makes my Jenkins job fail, even though everything worked. (If you're curious, this is why I went down the luigi_run road in the first place.)

I'm curious what the thinking is here. Someone took the time to write sys.exit(luigi_run()) in the luigi script, even though that construct cannot possibly do what it says it does. Likewise, someone chose to bury sys.exit() calls in retcodes.py, at least two call levels removed from the program's entry point, a very bad practice. Is there a specific reason it's structured this way? Would it not be better to have run_with_retcodes() and luigi_run() return their exit code values, so that the top-level sys.exit() does the work of setting the program's return code in exactly one place, as it should? Is there some other angle that I'm missing?

--

Noah

[Arash Rouhani]

Nice digging and discovery.

Do you mind fixing the issue? :)

Cheers,

Arash

--
You received this message because you are subscribed to the Google Groups "Luigi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to luigi-user+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Noah Yetter]

PR submitted https://github.com/spotify/luigi/pull/2350

(I very rarely do PRs on public projects, hopefully this is adequate...)

--

Noah

To unsubscribe from this group and stop receiving emails from it, send an email to luigi-user+...@googlegroups.com.