ncbi / pgap

NCBI Prokaryotic Genome Annotation Pipeline
Other
310 stars 90 forks source link

[BUG] Unclear error messages #224

Closed MrTomRod closed 2 years ago

MrTomRod commented 2 years ago

I tried to run pgap using Python's subprocess.run. I got return code 1, STDOUT='HOME' and no STDERR.

The reason was that on line 493 of pgap.py, os.environ['HOME'] is called, which raises an error because I didn't define the HOME variable. Because of lines 936-940, only 'HOME' is printed:

    except (Exception, KeyboardInterrupt) as exc:
        if args.debug:
            raise
        retcode = 1
        print(exc)

I propose that you change the code to something like this:

    except (Exception, KeyboardInterrupt) as exc:
        if args.debug:
            raise
        retcode = 1
        import traceback
        traceback.print_exc()

Then, the whole stack trace is printed and I would have quickly found the problem.

azat-badretdin commented 2 years ago

Thank you for your report, Thomas!

Let's address first the missing HOME issue.

We fixed the missing HOME problem for Windows platform by using USERPROFILE for these cases for August release. Looks like more fixes are needed.

What is the HOME equivalent on your platform (output of platform.system() call in Python)?

azat-badretdin commented 2 years ago

As for your traceback enhancement, looks reasonable to me. I opened an internal new feature ticket for this (See labels)

MrTomRod commented 2 years ago

I'm using Centos 7. HOME is defined, normally, except when I start a command through Python via subprocess.run. I just added this environment variable manually, that fixed the problem.

# Ups, I accidentally closed the issue, sorry!

azat-badretdin commented 2 years ago

I just added this environment variable manually

Note that this option (PGAP_INPUT_DIR) is documented in Wiki.

Glad it worked out for you, Thomas!

MrTomRod commented 2 years ago

True, but the error would have happened anyway:

class Setup:

    def __init__(self, args):
        ...
        if platform.system() == 'Windows':
            self.install_dir     = os.environ.get('PGAP_INPUT_DIR',os.environ['USERPROFILE']+'/.pgap')
        else:
            self.install_dir     = os.environ.get('PGAP_INPUT_DIR',os.environ['HOME']+'/.pgap')

os.environ['HOME'] is executed and will raise an error even if PGAP_INPUT_DIR is in os.environ.

To demonstrate, run this:

import os
print(os.environ['HOME'])  # should work
os.environ.get('HOME', os.environ['DOES_NOT_EXIST'])  # will produce error even though HOME exists
azat-badretdin commented 2 years ago

This will go in the coming release (in the next few days)