Skip to content

Do not fail on receiving non-ascii from Tango #975

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

Closed
wants to merge 1 commit into from

Conversation

thotypous
Copy link
Contributor

Currently, Autolab fails if it receives unicode (or any non-ascii characters) from Tango, e.g.

An Encoding::UndefinedConversionError occurred in assessments#autograde_done:

  "\xC3" from ASCII-8BIT to UTF-8
  app/controllers/assessment/autograde.rb:458:in `write'

This is easily fixed by opening the file for writing in binary mode. This way, data is saved exactly as received from Tango. I tested it with a feedback message encoded in UTF-8 and everything else works as expected.

@thotypous
Copy link
Contributor Author

thotypous commented Apr 20, 2018

Travis CI tests appear to be failing even before the changed code path is called. Is that normal?

Edit: yep, Travis CI tests seem to be broken. All PRs are failing with the same error.

@thotypous thotypous force-pushed the fix-nonascii-feedback branch 2 times, most recently from e527c97 to 448e710 Compare April 20, 2018 04:46
@thotypous thotypous force-pushed the fix-nonascii-feedback branch from 448e710 to f7d9cac Compare April 20, 2018 11:57
@devanshk
Copy link
Contributor

It appears to be a quirk where any non-contributor PR fails the CI test. @zyx-billy I believe you found the reason for this a while back, can you give more insight on that?

In any case, we'll take a look at this manually and test it out.

@zyx-billy
Copy link
Member

Yes, right now we use encrypted environment variables (for Tango access) in our test cases. These aren't visible to forked repos, so travis builds will fail. We really should change this in the future so everything's more self-contained, but right now what we do is we'll go through your PR manually.

@thotypous
Copy link
Contributor Author

Nice, thanks!

@devanshk devanshk requested a review from TheodorJ April 30, 2018 16:13
@fanpu fanpu mentioned this pull request Sep 20, 2021
5 tasks
@fanpu
Copy link
Contributor

fanpu commented Sep 20, 2021

Will be resolved with #1384

@fanpu fanpu closed this Sep 20, 2021
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.

4 participants