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

Avoid locale specific string.letters for job_name #4121

Merged
merged 1 commit into from May 30, 2017

Conversation

Projects
None yet
4 participants
@peterjc
Copy link
Contributor

commented May 29, 2017

The job_name sanitation can fail if the locale specific string.letters contains unexpected characters. e.g. Saravanaraj Ayyampalayam reported getting the following under locale en_US.iso885915

Traceback (most recent call last):
  File /.../galaxy-dist/lib/galaxy/jobs/runners/drmaa.py, line 397, in <genexpr>
    job_name = ''.join( x if x in ( string.letters + string.digits + '_' ) else '_' for x in job_name )
UnicodeDecodeError: 'ascii' codec can't decode byte 0xa6 in position 52: ordinal not in range(128)

See http://dev.list.galaxyproject.org/Issue-with-job-name-UnicodeDecodeError-in-drmaa-td4670784.html or https://lists.galaxyproject.org/pipermail/galaxy-dev/2017-May/025687.html

The downside of this proposed change would be to mask reasonable non ASCII letters from the job name on a well defined system with consistent locale settings for both Galaxy and the Cluster. But ASCII only seems safer.

Avoid locale specific string.letters for job_name
The job_name sanitation can fail if the locale specific
string.letters contains unexpected characters. e.g.
Saravanaraj Ayyampalayam reported getting the following
under locale en_US.iso885915

Traceback (most recent call last):
  File /.../galaxy-dist/lib/galaxy/jobs/runners/drmaa.py, line 397, in <genexpr>
    job_name = ''.join( x if x in ( string.letters + string.digits + '_' ) else '_' for x in job_name )
UnicodeDecodeError: 'ascii' codec can't decode byte 0xa6 in position 52: ordinal not in range(128)

See http://dev.list.galaxyproject.org/Issue-with-job-name-UnicodeDecodeError-in-drmaa-td4670784.html
or https://lists.galaxyproject.org/pipermail/galaxy-dev/2017-May/025687.html

The downside of this proposed change would be to mask
reasonable non ASCII letters from the job name on a
well defined system with consistent locale settings
for both Galaxy and the Cluster. But ASCII only seems
safer.

@galaxybot galaxybot added the triage label May 29, 2017

@galaxybot galaxybot added this to the 17.09 milestone May 29, 2017

@nsoranzo

This comment has been minimized.

Copy link
Member

commented May 29, 2017

👍

From https://docs.python.org/3.0/whatsnew/3.0.html#library-changes :

string.letters and its friends (string.lowercase and string.uppercase) are gone. Use string.ascii_letters etc. instead. (The reason for the removal is that string.letters and friends had locale-specific behavior, which is a bad idea for such attractively-named global “constants”.)

@martenson

This comment has been minimized.

Copy link
Member

commented May 30, 2017

Thanks @peterjc for both the devlist troubleshooting and this PR.

@martenson martenson merged commit aa7d701 into galaxyproject:dev May 30, 2017

5 checks passed

api test Build finished. 276 tests run, 0 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 150 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 34 tests run, 0 skipped, 0 failed.
Details
toolshed test Build finished. 579 tests run, 0 skipped, 0 failed.
Details
@peterjc

This comment has been minimized.

Copy link
Contributor Author

commented May 30, 2017

Thanks - I didn't realise this had been removed in Python 3, thanks for that info.

@peterjc peterjc deleted the peterjc:locales_string_letters branch May 30, 2017

@nsoranzo

This comment has been minimized.

Copy link
Member

commented May 30, 2017

There are a few more instances in the codebase I think, I'll fix them as part of #1715.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.