rabix / sbpack

Command line utility to pack and upload/download CWL to/from SB powered platform
Apache License 2.0
8 stars 5 forks source link

Exit Code 1 for Successful Packs #18

Closed dmiller15 closed 2 years ago

dmiller15 commented 3 years ago

Having an issue where sbpack will return a non-zero exit code when pushing apps. Example:

(sbpack) ➜  kf-jointgenotyping-workflow git:(dm-lower-xmx) ✗ sbpack default kfdrc-harmonization/kf-reference-pipeline/kfdrc-jointgenotyping-refinement-workflow workflow/kfdrc-jointgenotyping-refinement-workflow.cwl

sbpack v2020.10.05
Upload CWL apps to any Seven Bridges powered platform
(c) Seven Bridges 2020

Packing workflow/kfdrc-jointgenotyping-refinement-workflow.cwl
...
--
Recursing into step workflow/kfdrc-jointgenotyping-refinement-workflow.cwl:vep_annotate
<App: id=kfdrc-harmonization/kf-reference-pipeline/kfdrc-jointgenotyping-refinement-workflow rev=6>
(sbpack) ➜  kf-jointgenotyping-workflow git:(dm-lower-xmx) ✗ echo $?
1

This exit code characteristic is not universal for sbpack. Running with the help command gives the expected 0 exit code.

(sbpack) ➜  kf-jointgenotyping-workflow git:(dm-lower-xmx) ✗ sbpack -h

sbpack v2020.10.05
Upload CWL apps to any Seven Bridges powered platform
(c) Seven Bridges 2020

usage: sbpack [-h] [--filter-non-sbg-tags] profile appid cwl_path

positional arguments:
  profile               SB platform profile as set in the SB API credentials
                        file.
  appid                 Takes the form {user}/{project}/{app_id}.
  cwl_path              Path or URL to the main CWL file to be uploaded.

optional arguments:
  -h, --help            show this help message and exit
  --filter-non-sbg-tags
                        Filter out custom tags that are not 'sbg:'
(sbpack) ➜  kf-jointgenotyping-workflow git:(dm-lower-xmx) ✗ echo $?
0

I've reproduced this behavior in a docker environment with the most up to date version of sbpack and different cwl files.

In all it's not terribly critical as the packing and pushing still works, but it becomes problematic when trying to use the tool in any sort of automated testing. Tests that check the exit code indicate the task failed when it appears to have succeeded.

I was just wondering if this behavior was expected, and perhaps I'm missing something.

dmiller15 commented 3 years ago

Hey just following up with some investigation of my own.

If we interrogate what sbpack is running after it's installed we can see a pretty simple wrapper for the main function:

root@41463c4572c2:/sbpack# cat `which sbpack`
#!/usr/bin/python3
# -*- coding: utf-8 -*-
import re
import sys
from sbpack.pack import main
if __name__ == '__main__':
    sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
    sys.exit(main())

Basically taking the output, if any, from main() and using that as the argument for sys.exit. So what are we getting as an output from main()? https://github.com/rabix/sbpack/blob/master/sbpack/pack.py#L361-L367

    try:
        app = api.apps.get(appid)
        logger.debug("Creating revised app: {}".format(appid))
        return api.apps.create_revision(id=appid, raw=cwl, revision=app.revision + 1)
    except sbgerr.NotFound:
        logger.debug("Creating new app: {}".format(appid))
        return api.apps.install_app(id=appid, raw=cwl)

The main function can end in two ways, either providing the output of the sevenbridges.api.apps create_revision or install_app function. Looking at these then we can see both returns an app object: https://github.com/sbg/sevenbridges-python/blob/b3e14016066563470d978c9b13e1a236a41abea8/sevenbridges/models/app.py#L136 https://github.com/sbg/sevenbridges-python/blob/b3e14016066563470d978c9b13e1a236a41abea8/sevenbridges/models/app.py#L159

        return App(api=api, **app_wrapper)

So either way we have main() returning an App object. In retrospect this fact should be obvious as we can see the commands from the original issue returning the app as their last line:

<App: id=kfdrc-harmonization/kf-reference-pipeline/kfdrc-jointgenotyping-refinement-workflow rev=6>

What I assumed was a log is actually the output of sys.exit().

So why does this result in an exit code of 1? Well I think that's just due to the nature of sys.exit(): https://docs.python.org/3/library/sys.html#sys.exit. In the documentation for the code we can see that:

The optional argument arg can be an integer giving the exit status (defaulting to zero), or another type of object...If another type of object is passed, None is equivalent to passing zero, and any other object is printed to stderr and results in an exit code of 1.

Now the behavior makes sense: create_revision() or install_app() return the App to main() which returns that App as its output. The App from main() is then interpreted by the sys.exit() as the stderr so it prints it and returns exit code 1 indicating failure.

Possible resolution:

bogdang989 commented 3 years ago

Thanks @dmiller15 for reporting this and for the detailed investigation. Fix will be included in the next release

migbro commented 2 years ago

Hi @bogdang989 , is there a schedule for the next planned release? This bug is still an issue for us. Thanks!

bogdang989 commented 2 years ago

Hi @migbro sorry for the late response, I will see to push a new release in the next couple of days.

bogdang989 commented 2 years ago

Should be fixed with https://github.com/rabix/sbpack/releases/tag/2022.02.18

migbro commented 2 years ago

Great, thanks for that!