Remove migration downtime check since we do not allow downtime [RUN ALL RSPEC] [RUN AS-IF-FOSS]
What does this MR do?
For 4 years we've had logic in our database migrations that required you
to set a constant DOWNTIME = true
if you required downtime and seek
approval from the VP of Engineering. We have never once used this
process as we've always found a way around the problem using a different
approach.
As such we decided in
#326495 (closed) that we should just
remove this DOWNTIME
constant and the extra checks here to reduce
noise in our database code review and give less processes for people to
learn.
This MR removes a lot of things and here is a high level summary:
- Remove DowntimeCheck class and the rake task that invoked and the CI job that invoked that rake task and any related tests, helper classes
- Update documentation to make it clearer that downtime is not allowed and therefore remove the approval process
- Update a page called
what_requires_downtime
to be calledavoiding_downtime_in_migrations
since it was already a set of patterns to avoid downtime and now it's worded more strongly to imply that we can always get away without downtime - Various other docs pages that had examples of migrations that used
the
DOWNTIME
constant - Various rubocop specs that had migrations in them which used the
DOWNTIME
constant
The one thing I did not do is go back and remove DOWNTIME = false
from
all the existing migrations. In general we should not be updating
migrations once they've run and this would have made this MR change many
thousands of files so it's not worth it.
Screenshots (strongly suggested)
Does this MR meet the acceptance criteria?
Conformity
-
📋 Does this MR need a changelog?- [-] I have included a changelog entry.
-
I have not included a changelog entry because _____.
-
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. - [-] Tested in all supported browsers
- [-] Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
- [-] Label as security and @ mention
@gitlab-com/gl-security/appsec
- [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
- [-] Security reports checked/validated by a reviewer from the AppSec team
Related to #326495 (closed)