Document the migration "rules"

ManageIQ / manageiq-schema

SQL schema and database migrations for ManageIQ

Apache License 2.0

20 stars 125 forks source link

Document the migration "rules" #4

Closed Fryguy closed 3 years ago

Fryguy commented 7 years ago

Use say_with_time
All data migrations must have specs

lpichler commented 7 years ago

when you are working with the model in data migration, define the model in the migration
when there is model defined in your migration and it has column type, set self.inheritance_column = :_type_disabled example
```
class CloudNetwork < ActiveRecord::Base
self.inheritance_column = :_type_disabled
end
```

Fryguy commented 7 years ago

Describe the "why" behind having the rule that we don't use default, null, foreign keys, etc.

lpichler commented 7 years ago

don't mix data and structural migration to the one migration, create separate migrations

Fryguy commented 7 years ago

Explain the pitfalls of deserializing columns that might have serialized objects in them. In particular, MiqExpression, which is common, and describe avoiding desrialization and using a LIKE query can be used instead.
Describe the benefits of update_all
Describe batching and when it's possible to use a reentrant migration for LARGE data migrations.

miq-bot commented 6 years ago

This issue has been automatically marked as stale because it has not been updated for at least 6 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions!

lpichler commented 6 years ago

@miq-bot remove_label stale

Fryguy commented 6 years ago

We need a note about if remove_column appears in a PR, there needs to be a linked PR in other repos showing that those columns are no longer used.
Might wants to describe the different "hats" needed to be worn for review.
- DBA - review for database schema issues that could lead to performance/inefficiency problems, as well as index selection
- Rails - review for Rails-invalid column names such as non-foreign-keys that end in _id or a type column that isn't being used for STI
- ManageIQ - review for understanding where the new tables fits in the grand scheme and what use cases it is supporting. For providers, it's important to understand if the new tables and the naming of things fits into the single pane of glass, and how those new tables would be reused by other provider of the same provider type.

Fryguy commented 6 years ago

Migrations should not create new rows if rows didn't previously exist...that is, an empty database migrated should still be empty in the end, particularly for test reasons. Data migrations of existing data can transform into new rows if necessary.

bdunne commented 6 years ago

Be region-aware in data migrations. When a data migration runs in the global region, you probably only want to update records for your region (especially when the data migration is complicated). Updating records for other regions is a waste of time since the migration will be run in the remote region and it will push the changes up.

lpichler commented 5 years ago

Use ActiveRecord::Base for models in migrations.

Part of the advantage of ApplicationRecord is that we can leave AR::Base unmolested, which is exactly what migrations want: a plain ordinary AR object with no app-code-of-the-day surprises.

taken from https://github.com/ManageIQ/manageiq/pull/7312/files#r56356275