Closed cgilmour closed 8 years ago
Seems fine. In general, though, I like strings much better as the exposed IDs for resources. Currently, our IDs are based on the sequence numbers automatically created by the database. If we change storage systems then maybe we have UUIDs or other things as IDs and they would be strings again.
There are also a few other considerations around exposed IDs, which generally make people prefer 'random looking' ones, rather than those strictly increasing ones.
So, basically, I don't want to hold this up, so it can go ahead and be merged. In the long run, though, I would prefer for all exposed IDs to be strings, which can potentially be made to look like random IDs.
I'll send updates for making ID consistent and having strings on the exposed API. Will ping for another review after that.
I started some changes to make ID
consistent throughout (and IP
and CIDR
, but it leaks into other areas (eg: kube / networking) which would need updating to match. So I'll hold off on that part and just revert the ID type back to string.
Ok. Can you create a card for this, though? With a summary of the issues and concerns on there?
These changes are related to installs on Ubuntu 16.04, which was suffering from
Error 1071 (Specified key was too long; max key length is 767 bytes)
when creating the DB tables for Romana microservices.The cause is that strings are mapped to varchar(255) by
gorm
, which becomes 1,020 bytes when the DB server uses character setutf8mb4
, and exceeds the limit. The impact is on unique columns or unique indexes.The changes were branched off v0.8.2, and would need merging to master if accepted and put into a point release.
Tested on Ubuntu 14.04 / kubernetes with current
romana
installer, and on 16.04 / kubernetes with modified installer.