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

[17.09][Bug] Encode file content with utf-8 #4987

Merged
merged 2 commits into from Nov 13, 2017

Conversation

Projects
None yet
3 participants
@mvdbeek
Member

mvdbeek commented Nov 12, 2017

The previous syntax would fail if dataset_attrs['info'] is None, but I think it's cleaner to just specify the encoding when reading the file contents

There shouldn't be any disadvantages over doing this only for specific elements.
This also fixes #4345 (comment).
I have tested this with unicode strings in the name and info fields.

@nsoranzo does this make sense to you ?

Encode file content with utf-8
There shouldn't be any disadvantages over doing this only for specific elements.
This also fixes #4345 (comment).
I have tested this with unicode strings in the name and info fields.
@nsoranzo

This comment has been minimized.

Member

nsoranzo commented Nov 12, 2017

Every string read from a file using read_file_contents() in used only as input to loads(). Can we remove read_file_contents() and just use load(open(file_path)) , opening the file in text mode?

(the default encoding of json.load() is 'utf-8')

@jmchilton jmchilton merged commit a0b4a2d into galaxyproject:release_17.09 Nov 13, 2017

7 checks passed

api test Build finished. 292 tests run, 4 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 161 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 46 tests run, 0 skipped, 0 failed.
Details
lgtm analysis: JavaScript No alert changes
Details
selenium test Build finished. No test results found.
Details
toolshed test Build finished. 579 tests run, 0 skipped, 0 failed.
Details

@nsoranzo nsoranzo added this to the 18.01 milestone Nov 13, 2017

@galaxyproject galaxyproject deleted a comment from galaxybot Nov 13, 2017

@mvdbeek mvdbeek deleted the mvdbeek:history_import_bug branch Jun 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment