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

Smart string handling on the wsgi response -- this fixes uwsgi blanki… #2630

Merged
merged 4 commits into from Jul 21, 2016

Conversation

Projects
None yet
4 participants
@dannon
Copy link
Member

commented Jul 19, 2016

…ng out when handling endpoints that return unicode.

This would prevent problems like seen with #2628

Smart string handling on the wsgi response -- this fixes uwsgi blanki…
…ng out when handling endpoints that return unicode.
@dannon

This comment has been minimized.

Copy link
Member Author

commented Jul 19, 2016

(might be pushing one more commit to handle iterables that need encoding post-flattening)

@nsoranzo

This comment has been minimized.

Copy link
Member

commented Jul 19, 2016

If you add that commit, it may be better to include the call to smart_str() inside flatten(), since it is a generator and it is used only by the make_body_iterable() method.

@dannon

This comment has been minimized.

Copy link
Member Author

commented Jul 19, 2016

@nsoranzo yep. still waiting on tests to find locations :)

@dannon dannon added status/WIP and removed status/review labels Jul 20, 2016

@dannon dannon added status/review and removed status/WIP labels Jul 20, 2016

@dannon

This comment has been minimized.

Copy link
Member Author

commented Jul 20, 2016

@nsoranzo Should work now. Like I mentioned on IRC, I added a few dummy endpoints with iterable garbage to test that it packs and ships correctly, and it does, even though I never did run across a 'live' endpoint that uses this functionality.

In any event, strings coming from the webapp would need to be non-unicode, and this should prevent further errors like #2628 from happening.

@martenson

This comment has been minimized.

Copy link
Member

commented Jul 21, 2016

The failed tests are probably Jenkins running out of memory. :/ I need to lower the number of workers.

edit: I have lowered the number of executors from 8 to 5. Hope it helps.

@martenson

This comment has been minimized.

Copy link
Member

commented Jul 21, 2016

@galaxybot test this

@nsoranzo

This comment has been minimized.

Copy link
Member

commented Jul 21, 2016

api and framework tests pass locally, let's give Jenkins this last chance before merging.

@nsoranzo nsoranzo merged commit 423e85f into galaxyproject:dev Jul 21, 2016

3 of 4 checks passed

api test Build finished. 224 tests run, 0 skipped, 2 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 111 tests run, 0 skipped, 0 failed.
Details
toolshed test Build finished. 582 tests run, 0 skipped, 0 failed.
Details
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.