Skip to content
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

Document the migration "rules" #4

Closed
Fryguy opened this issue Jun 20, 2017 · 10 comments · Fixed by #562
Closed

Document the migration "rules" #4

Fryguy opened this issue Jun 20, 2017 · 10 comments · Fixed by #562
Assignees

Comments

@Fryguy
Copy link
Member

Fryguy commented Jun 20, 2017

  • Use say_with_time
  • All data migrations must have specs
@lpichler
Copy link
Contributor

  • 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
Copy link
Member Author

Fryguy commented Jun 23, 2017

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

@lpichler
Copy link
Contributor

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

@Fryguy
Copy link
Member Author

Fryguy commented Aug 29, 2017

  • 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
Copy link
Member

miq-bot commented Apr 2, 2018

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!

@miq-bot miq-bot added the stale label Apr 2, 2018
@lpichler
Copy link
Contributor

lpichler commented Apr 3, 2018

@miq-bot remove_label stale

@miq-bot miq-bot removed the stale label Apr 3, 2018
@Fryguy
Copy link
Member Author

Fryguy commented Apr 11, 2018

  • 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 Fryguy added the pinned label Apr 11, 2018
@Fryguy
Copy link
Member Author

Fryguy commented Aug 7, 2018

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
Copy link
Member

bdunne commented Aug 10, 2018

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
Copy link
Contributor

lpichler commented Nov 2, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants