hoffmangroup / segway

Application for semi-automated genomic annotation.
http://segway.hoffmanlab.org/
GNU General Public License v2.0
13 stars 7 forks source link

Segway silently ignores DRMAA if installed incorrectly #52

Closed EricR86 closed 8 years ago

EricR86 commented 8 years ago

Original report (BitBucket issue) by Eric Roberts (Bitbucket: ericr86, GitHub: ericr86).


In segway/cluster/init.py line 16:

if DRIVER_NAME_OVERRIDE == "local":
    from .local import ExitTimeoutException, JobState, Session
else:
    try:
        from drmaa import ExitTimeoutException, JobState, Session
    except (ImportError, RuntimeError):
        DRIVER_NAME_OVERRIDE = "local"  # no DRMAA available
        from .local import ExitTimeoutException, JobState, Session

If drmaa is at all configured or installed incorrectly, segway will run on the cluster locally without any mention of this happening. There needs to be a more informative output in how segway is running or if a RuntimeError it should state the problem with the drmaa installation.

EricR86 commented 8 years ago

Original comment by Michael Hoffman (Bitbucket: hoffman, GitHub: michaelmhoffman).


ImportError should probably print a message to sys.stderr noting that drmaa is not installed, continuing in local mode.

Why is RuntimeError a fallback to local mode without printing an error? Blame says I added it, but it would have been adapted from a patch by @mlibbrecht. Max, did yours have RuntimeError in there? Why? Seems like we would want other sorts of errors to be overriden explicitly with SEGWAY_CLUSTER.

EricR86 commented 8 years ago

Original comment by Eric Roberts (Bitbucket: ericr86, GitHub: ericr86).


It's worth noting that the ImportError is unlikely to occur (but still possible I'd imagine) since it's installed from install_requires when installing Segway.

The RuntimeError catches the case where DRMAA might be misconfigured or not even installed on the system. For example not setting DRMAA_LIBRARY_PATH correctly.

EricR86 commented 8 years ago

Original comment by Michael Hoffman (Bitbucket: hoffman, GitHub: michaelmhoffman).


OK, good point, sounds like we should print a warning in all cases when the SEGWAY_CLUSTER environment variable is not defined. Question is whether we should just print a friendly warning or print the traceback too. You can see the previous version of this code before I added in Max's patch to see how you can print the traceback.

EricR86 commented 8 years ago

Original comment by Michael Hoffman (Bitbucket: hoffman, GitHub: michaelmhoffman).


A traceback would make it easier to diagnose problems but it would seem less friendly. That said, one could always disable it by setting SEGWAY_CLUSTER=local.

EricR86 commented 8 years ago

Original comment by Eric Roberts (Bitbucket: ericr86, GitHub: ericr86).


Why not just print a warning in the case where SEGWAY_CLUSTER is not defined and DRMAA fails to load?

I'm not sure if a traceback is necessary since the majority of cases will be where DRMAA is not installed. A simple warning (stderr) message of "DRMAA failed to load. Running segway locally." would be fine in my opinion.

EricR86 commented 8 years ago

Original comment by mlibbrecht (Bitbucket: mlibbrecht, GitHub: mlibbrecht).


I don't think I added the DRIVER_NAME_OVERRIDE part. In my branch, Segway just fails with an ImportError when SEGWAY_CLUSTER is not defined and DRMAA doesn't load. I think printing a warning when that happens is a good idea -- currently people often forget to set SEGWAY_CLUSTER and then get very confused by the DRMAA error message.

EricR86 commented 8 years ago

Original comment by Michael Hoffman (Bitbucket: hoffman, GitHub: michaelmhoffman).


Suggested warning: "Error loading drmaa. Falling back to local cluster mode. Set SEGWAY_CLUSTER=local to suppress this warning."

EricR86 commented 8 years ago

Original comment by Eric Roberts (Bitbucket: ericr86, GitHub: ericr86).


Resolved in Pull Request #57