Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix python3 string/bytes error when using --printLogs #3005

Merged

Conversation

johnbradley
Copy link
Contributor

Adds decode('utf-8') method call to fix the following error:

  .../toil/utils/toilStatus.py", line 83, in printJobLog
    msg += fH.read()
TypeError: can only concatenate str (not "bytes") to str

This fix is already being used in leader.py FailedJobsException:

toil/src/toil/leader.py

Lines 79 to 80 in 993be0c

with job.getLogFileHandle(jobStore) as fH:
msg += fH.read().decode('utf-8')

Let me know if you would prefer a new function to abstract out this duplication.

For testing the bug I added an example CWL workflow that always fails.

Fixes problem raised in #3004

Copy link
Member

@adamnovak adamnovak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! I'll pull it in for testing.

Copy link
Member

@DailyDreaming DailyDreaming left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnbradley Thanks for putting in this fix!

@adamnovak
Copy link
Member

The Gitlab tests are all failing due to the absence of the pytz module. It isn't installed because installing our dependencies bails out, because http-parser can't install, becuase it goes looking for setuptools.Feature and can't find it.

Collecting http-parser
  Downloading http-parser-0.8.3.tar.gz (83 kB)
    ERROR: Command errored out with exit status 1:
     command: /builds/databiosphere/toil/venv/bin/python3.6 -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/pip-install-1bte35b_/http-parser/setup.py'"'"'; __file__='"'"'/tmp/pip-install-1bte35b_/http-parser/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' egg_info --egg-base /tmp/pip-install-1bte35b_/http-parser/pip-egg-info
         cwd: /tmp/pip-install-1bte35b_/http-parser/
    Complete output (5 lines):
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-install-1bte35b_/http-parser/setup.py", line 19, in <module>
        from setuptools import setup, find_packages, Extension, Feature
    ImportError: cannot import name 'Feature'
    ----------------------------------------
ERROR: Command errored out with exit status 1: python setup.py egg_info Check the logs for full command output.

Between the last successful job and this one we changed from setuptools 45 to 56. In 46 they removed Feature (see https://github.com/pypa/setuptools/blob/master/CHANGES.rst#v4600 and pypa/setuptools#65); now http-parser is completely broken and not installable.

We either need to make Toil require setuptools <46, or fix http-parser and pymesos.

Test to reproduce `TypeError: can only concatenate str (not "bytes") to str` error.
@adamnovak adamnovak force-pushed the issues/3004-python3-printLogs branch from 502343d to 014dcba Compare March 11, 2020 21:34
@adamnovak
Copy link
Member

OK I stacked this on top of current master which passed CI; I think this will pass too.

@adamnovak adamnovak merged commit 2bd7920 into DataBiosphere:master Mar 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants