-
-
Notifications
You must be signed in to change notification settings - Fork 614
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
database: No longer store or retrieve InitialIP #7942
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Naively I'd expect this change to include a db schema change that drops this column from db-next -- is there a reason we're not doing that yet?
The path forward for removing the column didn’t feel solid enough to justify a schema change at this time. However, if you support modifying the table now, I’m happy to create another PR with the following migration: -- +migrate Up
-- SQL in section 'Up' is executed when this migration is applied
ALTER TABLE `registrations`
DROP COLUMN `initialIP`,
DROP KEY `initialIP_createdAt`;
-- +migrate Down
-- SQL section 'Down' is executed when this migration is rolled back
ALTER TABLE `registrations`
ADD COLUMN `initialIP` binary(16) NOT NULL DEFAULT '\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0',
ADD KEY `initialIP_createdAt` (`initialIP`, `createdAt`); |
Yeah, I think doing that schema change now would be good, as "proof" that it's safe for SRE to drop the column (however they choose to do so) in Staging and then in Prod. |
The initialIP column has been defaulted to 0.0.0.0 since #7760. Remove this field from the all structs while leaving the schema itself intact.
Part of #7917