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

Add support for emojis #64

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

nbey
Copy link
Contributor

@nbey nbey commented Nov 19, 2018

  • Allow charset for mySQL to be configured in /emergence/config.json file

@nbey
Copy link
Contributor Author

nbey commented Nov 19, 2018

@themightychris Could you take a look at this when you get a chance?

@themightychris
Copy link
Member

themightychris commented Nov 19, 2018

@nbey what does charset need to be set to to support emojis? Is there any downsides to switching the default to that?

@nafisbey
Copy link

@themightychris Charset needs to be set from utf8 -> to utf8mb4.

According to this article, we should probably just update the default to utf8mb4, since utf8 isn't actually UTF-8.

@themightychris
Copy link
Member

@nbey thanks!

It sounds like the consensus is everyone should be using utf8mb4 all the time, so we should try to propagate a wide switchover to it

All the guides I've found though discuss changing the server/client config and migrating the tables in sync, I haven't been able to find any clarity on what would happen if we just changed the client/server config now and migrated tables independently.

It seems like these settings just set the default for new tables, and I know MySQL thoroughly supports having tables and columns in various character sets during runtime. So my gut is that it would be fine to just switch the default now and roll out a migration in skeleton to convert existing tables whenever admins want to pull the trigger on that

However if we find any evidence that changing config and migrating tables out of sync would be problematic, we might add support to the kernel to run system-level migrations so it could migrate all tables next time it starts up with the new default, recording state in /emergence/config.json

Any thoughts from your own experiments and reading?

@nbey
Copy link
Contributor Author

nbey commented Nov 20, 2018

@themightychris Yeah, I think we could go with the former, and just create site-level migrations.

I tested these changes on my development server; first I made the changes to the mysql config file, then I restarted the server. Then I tried saving an emoji, which failed as expected until I migrated the database/table, so I think it should be fine.

Before we merge though, I can run a few more tests around having a utf8 charset for db/table/column with the server/client charset set to utf8mb4.

@themightychris
Copy link
Member

@nbey that would be really helpful, please do

And this can be a sep issue, but do you happen to have any code lying around still for looping over tables to apply migration?

@nbey
Copy link
Contributor Author

nbey commented Nov 20, 2018

@themightychris I don't but I can look into writing up a quick migration script; we shouldn't have to do more than update the charset/collation for the db/tables, and then repair/optimize the tables.

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.

3 participants