From 30dbbe3870b28be1b6696f653adec29e10b4c07a Mon Sep 17 00:00:00 2001
From: Thong Kuah <tkuah@gitlab.com>
Date: Wed, 13 Jul 2022 13:35:07 +1200
Subject: [PATCH] Remove branch / MR creation from LFK generation script
This was something we required for en-masse creation but we do not need
to maintain this going forward
---
.../database/loose_foreign_keys.md | 4 -
.../decomposition/generate-loose-foreign-key | 87 -------------------
2 files changed, 91 deletions(-)
diff --git a/doc/development/database/loose_foreign_keys.md b/doc/development/database/loose_foreign_keys.md
index dec51d484fd6d..6aa1b9c40ff0f 100644
--- a/doc/development/database/loose_foreign_keys.md
+++ b/doc/development/database/loose_foreign_keys.md
@@ -66,8 +66,6 @@ The tool ensures that all aspects of swapping a foreign key are covered. This in
- Updating `db/structure.sql` with the new migration.
- Updating `lib/gitlab/database/gitlab_loose_foreign_keys.yml` to add the new loose foreign key.
- Creating or updating a model's specs to ensure that the loose foreign key is properly supported.
-- Creating a new branch, commit, push, and creating a merge request on GitLab.com.
-- Creating a merge request template with all the necessary details to validate the safety of the foreign key removal.
The tool is located at `scripts/decomposition/generate-loose-foreign-key`:
@@ -77,9 +75,7 @@ $ scripts/decomposition/generate-loose-foreign-key -h
Usage: scripts/decomposition/generate-loose-foreign-key [options] <filters...>
-c, --cross-schema Show only cross-schema foreign keys
-n, --dry-run Do not execute any commands (dry run)
- -b, --[no-]branch Create or not a new branch
-r, --[no-]rspec Create or not a rspecs automatically
- -m, --milestone MILESTONE Specify custom milestone (current: 14.8)
-h, --help Prints this help
```
diff --git a/scripts/decomposition/generate-loose-foreign-key b/scripts/decomposition/generate-loose-foreign-key
index 35f84c64ce167..f20712a025880 100755
--- a/scripts/decomposition/generate-loose-foreign-key
+++ b/scripts/decomposition/generate-loose-foreign-key
@@ -6,10 +6,8 @@
require 'optparse'
$options = {
- milestone: "#{Gitlab.version_info.major}.#{Gitlab.version_info.minor}",
cross_schema: false,
dry_run: false,
- branch: true,
rspec: true
}
@@ -24,18 +22,10 @@ OptionParser.new do |opts|
$options[:dry_run] = v
end
- opts.on("-b", "--[no-]branch", "Create or not a new branch") do |v|
- $options[:branch] = v
- end
-
opts.on("-r", "--[no-]rspec", "Create or not a rspecs automatically") do |v|
$options[:rspec] = v
end
- opts.on("-m", "--milestone MILESTONE", "Specify custom milestone (current: #{$options[:milestone]})") do |v|
- $options[:milestone] = v
- end
-
opts.on("-h", "--help", "Prints this help") do
puts opts
exit
@@ -151,8 +141,6 @@ def add_definition_to_yaml(definition)
Rails.root.join('config/gitlab_loose_foreign_keys.yml'),
content.to_yaml.gsub(/^([- ] )/, ' \1')
)
-
- exec_cmd("git", "add", "config/gitlab_loose_foreign_keys.yml")
end
def generate_migration(definition)
@@ -187,10 +175,7 @@ def generate_migration(definition)
File.write(migration_name, content)
- exec_cmd("git", "add", migration_name, fail: "Failed to add migration file.")
exec_cmd("bin/rails", "db:migrate", fail: "Failed to run db:migrate.")
- exec_cmd("git", "add", "db/schema_migrations/#{timestamp}", "db/structure.sql", fail: "There are uncommitted changes. We should not have any.")
- exec_cmd("git diff --exit-code --name-only", fail: "There are uncommitted changes. We should not have any.")
end
def class_by_table_name
@@ -257,8 +242,6 @@ def add_test_to_specs(definition)
break
end
end
-
- exec_cmd("git", "add", spec_path, fail: "There are uncommitted changes. We should not have any.")
end
def update_no_cross_db_foreign_keys_spec(definition)
@@ -275,75 +258,6 @@ def update_no_cross_db_foreign_keys_spec(definition)
end
File.write(spec_path, updated.join(""))
- exec_cmd("git", "add", spec_path, fail: "Failed to add changes from #{spec_path}")
-end
-
-def commit_changes(definition)
- branch_name = "remove-#{definition.to_table}_#{definition.from_table}_#{definition.column}-fk"
- commit_title = "Swap FK #{definition.from_table} to #{definition.to_table} for LFK"
- mr_title = "Swap FK #{definition.from_table}.#{definition.column} to #{definition.to_table} for LFK"
- description = <<-EOF.strip_heredoc
- Swaps FK for #{definition.from_table}.#{definition.column} to #{definition.to_table}
-
- Changelog: changed
- EOF
-
- commit_message = "#{commit_title}\n\n#{description}"
-
- existing_branch = %x[git rev-parse --abbrev-ref HEAD].strip
-
- if $options[:branch]
- unless exec_cmd("git", "checkout", "-b", branch_name)
- raise "Failed to create branch: #{branch_name}"
- end
- end
-
- unless exec_cmd("git", "commit", "-m", commit_message)
- raise "Failed to commit changes."
- end
-
- if $options[:branch]
- exec_cmd("git", "push", "origin", "-u", "HEAD",
- "-o", "merge_request.create",
- "-o", "merge_request.target=#{existing_branch}",
- "-o", "merge_request.milestone=#{$options[:milestone]}",
- "-o", "merge_request.title=#{mr_title}"
- )
-
- puts
- puts "--------------------------------------------------"
- puts "Put this as MR description:"
- puts "--------------------------------------------------"
- puts <<-EOF.strip_heredoc
- ## What does this MR do and why?
-
- Per https://gitlab.com/groups/gitlab-org/-/epics/7249
-
- As part of our CI "decomposition" efforts we need to remove all foreign keys that are cross-database (ie. between the planned \`main\` and \`ci\` databases). We are going to replace them all with ["loose foreign keys"](https://docs.gitlab.com/ee/development/database/loose_foreign_keys.html).
-
- Related: <DETAIL>
-
- ## Validations
-
- - **Best team to review (check off when reviewed):** TBD
- - [ ] No way for user to access once parent is deleted. Please explain: <DETAIL>
- - [ ] Possible to access once parent deleted but low user impact. Please explain: <DETAIL>
- - [ ] Possible Sidekiq workers that may load directly and possibly lead to exceptions. Please explain: <DETAIL>
- - [ ] Possible user impact to be evaluated or mitigated. Please explain: <DETAIL>
- - [ ] Is this FK safe to be removed to avoid LOCKing problems? (Explanation: https://gitlab.com/groups/gitlab-org/-/epics/7249#note_819662046). Please explain: <DETAIL>
-
- ## MR acceptance checklist
-
- This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
- * [ ] I have evaluated the [MR acceptance checklist](https://docs.gitlab.com/ee/development/code_review.html#acceptance-checklist) for this MR.
-
- /label ~"ci-decomposition::phase4" ~"database::review pending" ~"devops::enablement" ~"group::sharding" ~"section::enablement" ~"sharding::active" ~"type::feature" ~"workflow::in dev" ~backend ~"ci-decomposition" ~database ~"Category:Sharding"
- /milestone %"#{$options[:milestone]}"
- /assign_reviewer @ahegyi
- EOF
- puts "--------------------------------------------------"
- end
end
all_foreign_keys = ActiveRecord::Base.connection.tables.flat_map do |table|
@@ -400,6 +314,5 @@ all_foreign_keys.each_with_index do |definition, idx|
generate_migration(definition)
add_test_to_specs(definition)
update_no_cross_db_foreign_keys_spec(definition)
- commit_changes(definition)
end
puts
--
GitLab