Modify all models + migrations to have stripe_id be VARCHAR(191)

[ahubers]

Motivation

The max length for a key field in MySQL is 767 Bytes. A VARCHAR(255) in utf8 does not exceed that limit, however it does with bulkier encodings like utf8mb4. These encodings are becoming more common as they support emojis.

Reducing the stripe_id field to VARCHAR(191) keeps us from hitting that limit.

For some context on the issue I'm having, trying to run pinax-stripe migrations on our django instance is resulting in the following OperationalError:

self = <_mysql.connection open to '192.168.4.23' at 2dbf798>
query = b'CREATE TABLE `pinax_stripe_bitcoinreceiver` (`id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY, `stripe_id` varchar(2...text NOT NULL, `refund_address` longtext NOT NULL, `uncaptured_funds` bool NOT NULL, `used_for_payment` bool NOT NULL)'

    def query(self, query):
        # Since _mysql releases GIL while querying, we need immutable buffer.
        if isinstance(query, bytearray):
            query = bytes(query)
        if self.waiter is not None:
            self.send_query(query)
            self.waiter(self.fileno())
            self.read_query_result()
        else:
>           _mysql.connection.query(self, query)
E           django.db.utils.OperationalError: (1071, 'Specified key was too long; max key length is 767 bytes')

/home/vagrant/getmyboat/lib/python3.4/site-packages/MySQLdb/connections.py:270: OperationalError

Additional Info

The Motivation section above is informed entirely by https://stackoverflow.com/questions/1814532/1071-specified-key-was-too-long-max-key-length-is-767-bytes.

But Alex, you modified a migration? Isn't that bad?

Patching the migration with a new one would still result in the same failure I show above because it will still try to create the tables with the VARCHAR(255) keys. I don't see a problem with modifying these because I don't see stripe IDs ever having a length > 191. So this shouldn't affect any clients' tables.

For those already using pinax-stripe in production environments, they won't notice any difference -- for those trying to get it started in their own environment, they shouldn't run into this problem now...

[ahubers]

This also Closes #273.

[gregnewman]

LGTM @paltman

[paltman]

@brosner I'd love to hear your thoughts on this.

[tstirrat15]

What's the status on this?

[brosner]

@paltman it'd be best for me not to discuss my thoughts on this issue :-)

Database preferences aside, I accept this change. Thank you @ahubers.

[tstirrat15]

@paltman can this now be merged?

[paltman]

@brosner cool. I'll I'll merge it. Should this be considered BIC? What will happen when people upgrade and there are changes to migrations that are already applied?

[paltman]

Thank you for your patience @ahubers as well as your contribution.

[tstirrat15]

@paltman when can we expect this to be released?

[gregnewman]

@tstirrat15 we are close. I'm finishing up some documentation and I think there's an issue that needs tests before he pushes a new release. Most of the work on the current milestone is done.

[ahubers]

What will happen when people upgrade and there are changes to migrations that are already applied?

The migration will already exist in their django_migrations table and nothing will happen for those users. If they have build pipelines that rerun migrations from scratch for their test suite (something like... Django), then their test suite will test against a DB created with this new schema. However I don't see that causing issues unless they are inserting unnecessarily long strings into stripe_id fields in their test fixtures.