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 one more unicode error. Modify test so these errors don't recur in the future. Change color of admin banner page to differentiate it from the main server. #2512

Merged
merged 1 commit into from
Sep 26, 2016

Conversation

seanlip
Copy link
Member

@seanlip seanlip commented Sep 26, 2016

@BenHenning: Note that this change clears all remaining errors (at least for the exploration I tested locally).

…n the future. Change color of admin banner page to differentiate it from the main server.
Copy link
Member

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for doing this! I had one question.

@@ -1240,8 +1240,8 @@ def reduce(key, stringified_values):
yield (
'Item ID: %s, last updated: %s, state name: %s, '
'exp id: %s, error: %s' % (
item_id, last_updated, state_name, exploration_id,
error))
item_id, last_updated, state_name.encode('utf-8'),
Copy link
Member

Choose a reason for hiding this comment

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

Should we also be encoding state_name when it's being logged to the new storage model?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so. We only need to encode it when we are displaying it. The state_name that's being logged to the storage model is the same that is logged to the 'states' property of an Exploration storage model.

@BenHenning BenHenning merged commit afdc830 into answer-migration Sep 26, 2016
@BenHenning BenHenning deleted the answer-migration-errors branch September 26, 2016 02:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants