DEV: Refactor migrations-tooling

* Updates GitHub Action for migrations
* Rubocop: Always `EnforcedShorthandSyntax` for hashes in the `migrations` directory
* Automatically load all available converter steps
* Enable YJIT at runtime, if available
* Progressbar shows skipped records and other small improvements
This commit is contained in:
Gerhard Schlager
2025-03-30 22:36:50 +02:00
committed by Gerhard Schlager
parent 7b5839ec44
commit 71a90dcba2
27 changed files with 258 additions and 201 deletions

View File

@ -23,21 +23,16 @@ permissions:
jobs:
tests:
if: github.event_name == 'pull_request' || github.repository != 'discourse/discourse-private-mirror'
name: Tests with Ruby ${{ matrix.ruby }}
runs-on: ${{ (github.repository != 'discourse/discourse' && 'ubuntu-latest') || 'debian-12' }}
container: discourse/discourse_test:slim
name: Tests
runs-on: ${{ (github.repository_owner == 'discourse' && 'debian-12') || 'ubuntu-latest' }}
container: discourse/discourse_test:release
timeout-minutes: 20
env:
RAILS_ENV: test
PGUSER: discourse
PGPASSWORD: discourse
strategy:
fail-fast: false
matrix:
ruby: ["3.3"]
LOAD_PLUGINS: 1
steps:
- name: Set working directory owner
@ -62,23 +57,9 @@ jobs:
sudo -E -u postgres script/start_test_db.rb
sudo -u postgres psql -c "CREATE ROLE $PGUSER LOGIN SUPERUSER PASSWORD '$PGPASSWORD';"
- name: Container envs
id: container-envs
- name: Symlink vendor/bundle from image
run: |
echo "ruby_version=$RUBY_VERSION" >> $GITHUB_OUTPUT
echo "debian_release=$DEBIAN_RELEASE" >> $GITHUB_OUTPUT
shell: bash
- name: Bundler cache
uses: actions/cache@v4
with:
path: vendor/bundle
key: >-
${{ runner.os }}-
${{ steps.container-envs.outputs.ruby_version }}-
${{ steps.container-envs.outputs.debian_release }}-
${{ hashFiles('**/Gemfile.lock') }}-
migrations-tooling
ln -s /var/www/discourse/vendor/bundle vendor/bundle
- name: Setup gems
run: |
@ -88,11 +69,14 @@ jobs:
bundle config --local without development
bundle config --local with migrations
bundle install --jobs $(($(nproc) - 1))
bundle clean
- name: pnpm install
run: pnpm install --frozen-lockfile
- name: Get CPU cores
id: cpu-info
run: echo "cpu-cores=$(nproc)" >> $GITHUB_OUTPUT
- name: Fetch app state cache
uses: actions/cache@v4
id: app-cache
@ -100,7 +84,8 @@ jobs:
path: tmp/app-cache
key: >-
${{ runner.os }}-
${{ hashFiles('.github/workflows/tests.yml') }}-
${{ steps.cpu-info.outputs.cpu-cores }}-
${{ hashFiles('.github/workflows/migration-tests.yml') }}-
${{ hashFiles('db/**/*', 'plugins/**/db/**/*') }}-
${{ hashFiles('config/environments/test.rb') }}

4
migrations/.rubocop.yml Normal file
View File

@ -0,0 +1,4 @@
inherit_from: ../.rubocop.yml
Style/HashSyntax:
EnforcedShorthandSyntax: always

View File

@ -1,7 +1,7 @@
#!/usr/bin/env ruby
# frozen_string_literal: true
require_relative "../lib/migrations"
require_relative "../migrations"
require "colored2"
require "thor"
@ -44,6 +44,12 @@ module Migrations
end
end
if defined?(RubyVM::YJIT)
RubyVM::YJIT.enable
else
warn "WARNING: Performance degraded: RubyVM::YJIT is not available".yellow
end
# rubocop:disable Discourse/NoChdir
Dir.chdir(File.expand_path("../..", __dir__)) { ::Migrations::CLI::Application.start }
# rubocop:enable Discourse/NoChdir

View File

@ -1,5 +1,8 @@
en:
progressbar:
skips:
one: "%{count} skip"
other: "%{count} skips"
warnings:
one: "%{count} warning"
other: "%{count} warnings"

View File

@ -2,21 +2,16 @@
module Migrations
module DateHelper
# based on code from https://gist.github.com/emmahsax/af285a4b71d8506a1625a3e591dc993b
def self.human_readable_time(secs)
return "< 1 second" if secs < 1
[[60, :seconds], [60, :minutes], [24, :hours], [Float::INFINITY, :days]].map do |count, name|
next if secs <= 0
secs, number = secs.divmod(count)
unless number.to_i == 0
"#{number.to_i} #{number == 1 ? name.to_s.delete_suffix("s") : name}"
def self.human_readable_time(seconds)
hours, remainder = seconds.divmod(3600)
minutes, seconds = remainder.divmod(60)
format("%02d:%02d:%02d", hours, minutes, seconds)
end
end
.compact
.reverse
.join(", ")
def self.track_time
start_time = Time.now
yield
Time.now - start_time
end
end
end

View File

@ -1,5 +1,6 @@
# frozen_string_literal: true
require "colored2"
require "ruby-progressbar"
module Migrations
@ -7,9 +8,10 @@ module Migrations
def initialize(max_progress: nil)
@max_progress = max_progress
@skip_count = 0
@warning_count = 0
@error_count = 0
@extra_information = ""
@extra_information = +""
@base_format = nil
@progressbar = nil
@ -18,60 +20,58 @@ module Migrations
def run
raise "ProgressBar already started" if @progressbar
format = setup_progressbar
format = calculate_format
setup_progressbar
yield self
finalize_progressbar(format)
nil
end
def update(progress, warning_count, error_count)
extra_information_changed = false
def update(increment_by:, skip_count: 0, warning_count: 0, error_count: 0)
updated = false
if skip_count > 0
@skip_count += skip_count
updated = true
end
if warning_count > 0
@warning_count += warning_count
extra_information_changed = true
updated = true
end
if error_count > 0
@error_count += error_count
extra_information_changed = true
updated = true
end
if extra_information_changed
@extra_information = +""
update_format if updated
if @warning_count > 0
@extra_information << " | " <<
I18n.t("progressbar.warnings", count: @warning_count).yellow
end
if @error_count > 0
@extra_information << " | " << I18n.t("progressbar.errors", count: @error_count).red
end
@progressbar.format = "#{@base_format}#{@extra_information}"
end
if progress == 1
if increment_by == 1
@progressbar.increment
else
@progressbar.progress += progress
@progressbar.progress += increment_by
end
end
private
def setup_progressbar
format =
def calculate_format
if @max_progress
I18n.t("progressbar.processed.progress_with_max", current: "%c", max: "%C")
format = I18n.t("progressbar.processed.progress_with_max", current: "%c", max: "%C")
@base_format = " %a | %E | #{format}"
else
I18n.t("progressbar.processed.progress", current: "%c")
format = I18n.t("progressbar.processed.progress", current: "%c")
@base_format = " %a | #{format}"
end
@base_format = @max_progress ? " %a |%E | #{format}" : " %a | #{format}"
format
end
def setup_progressbar
@progressbar =
::ProgressBar.create(
total: @max_progress,
@ -83,14 +83,23 @@ module Migrations
format: @base_format,
throttle_rate: 0.5,
)
end
format
def update_format
@extra_information.clear
messages = []
messages << I18n.t("progressbar.skips", count: @skip_count).cyan if @skip_count > 0
messages << I18n.t("progressbar.warnings", count: @warning_count).yellow if @warning_count > 0
messages << I18n.t("progressbar.errors", count: @error_count).red if @error_count > 0
@extra_information << " | #{messages.join(" | ")}" unless messages.empty?
@progressbar.format = "#{@base_format}#{@extra_information}"
end
def finalize_progressbar(format)
print "\033[K" # delete the output of progressbar, because it doesn't overwrite longer lines
final_format = @max_progress ? " %a | #{format}" : " %a | #{format}"
@progressbar.format = "#{final_format}#{@extra_information}"
@progressbar.format = " %a | #{format}#{@extra_information}"
@progressbar.finish
end
end

View File

@ -30,7 +30,14 @@ module Migrations::Converters::Base
end
def steps
raise NotImplementedError
step_class = ::Migrations::Converters::Base::Step
current_module = self.class.name.deconstantize.constantize
current_module
.constants
.map { |c| current_module.const_get(c) }
.select { |klass| klass.is_a?(Class) && klass < step_class }
.sort_by(&:to_s)
end
def before_step_execution(step)
@ -70,7 +77,7 @@ module Migrations::Converters::Base
end
def create_step(step_class)
default_args = { settings: settings }
default_args = { settings: }
args = default_args.merge(step_args(step_class))
step_class.new(StepTracker.new, args)

View File

@ -39,7 +39,11 @@ module Migrations::Converters::Base
with_progressbar do |progressbar|
@step.items.each do |item|
stats = job.run(item)
progressbar.update(stats.progress, stats.warning_count, stats.error_count)
progressbar.update(
increment_by: stats.progress,
warning_count: stats.warning_count,
error_count: stats.error_count,
)
end
end
end
@ -90,7 +94,11 @@ module Migrations::Converters::Base
::Migrations::Database::IntermediateDB.insert(sql, *parameters)
end
progressbar.update(stats.progress, stats.warning_count, stats.error_count)
progressbar.update(
increment_by: stats.progress,
warning_count: stats.warning_count,
error_count: stats.error_count,
)
end
end
end

View File

@ -36,12 +36,7 @@ module Migrations::Converters::Base
private
def log(type, message, exception: nil, details: nil)
::Migrations::Database::IntermediateDB::LogEntry.create!(
type:,
message:,
exception:,
details:,
)
::Migrations::Database::IntermediateDB::LogEntry.create(type:, message:, exception:, details:)
end
end
end

View File

@ -6,7 +6,7 @@ module Migrations::Converters::Example
def execute
super
IntermediateDB::LogEntry.create!(type: "info", message: "This is a test")
IntermediateDB::LogEntry.create(type: "info", message: "This is a test")
end
end
end

View File

@ -14,7 +14,7 @@ module Migrations::Converters::Example
step.log_warning("Test", details: item) if item.in?([3, 7, 9])
step.log_error("Test", details: item) if item.in?([6, 10])
IntermediateDB::LogEntry.create!(type: "info", message: "Step2 - #{item}")
IntermediateDB::LogEntry.create(type: "info", message: "Step2 - #{item}")
end
end
end

View File

@ -17,7 +17,7 @@ module Migrations::Converters::Example
step.log_warning("Test", details: item) if item[:counter] > 10 && item[:counter] < 20
IntermediateDB::LogEntry.create!(type: "info", message: "Step3 - #{item[:counter]}")
IntermediateDB::LogEntry.create(type: "info", message: "Step3 - #{item[:counter]}")
end
end
end

View File

@ -59,5 +59,17 @@ module Migrations
return nil if value.nil?
::Oj.dump(value, mode: :compat)
end
def self.to_date(text)
text.present? ? Date.parse(text) : nil
end
def self.to_datetime(text)
text.present? ? DateTime.parse(text) : nil
end
def self.to_boolean(value)
value == 1
end
end
end

View File

@ -8,6 +8,7 @@ module Migrations::Database
PREPARED_STATEMENT_CACHE_SIZE = 5
def self.open_database(path:)
path = File.expand_path(path, ::Migrations.root_path)
FileUtils.mkdir_p(File.dirname(path))
db = ::Extralite::Database.new(path)
@ -25,7 +26,7 @@ module Migrations::Database
attr_reader :db, :path
def initialize(path:, transaction_batch_size: TRANSACTION_BATCH_SIZE)
@path = path
@path = File.expand_path(path, ::Migrations.root_path)
@transaction_batch_size = transaction_batch_size
@db = self.class.open_database(path:)
@statement_counter = 0
@ -54,24 +55,34 @@ module Migrations::Database
if (@statement_counter += 1) >= @transaction_batch_size
commit_transaction
end
end
def query(sql, *parameters, &block)
@db.query(sql, *parameters, &block)
end
def count(sql, *parameters)
@db.query_single_splat(sql, *parameters)
end
def execute(sql, *parameters)
@db.execute(sql, *parameters)
end
def begin_transaction
@db.execute("BEGIN DEFERRED TRANSACTION") unless @db.transaction_active?
end
def commit_transaction
if @db.transaction_active?
@db.execute("COMMIT")
@statement_counter = 0
end
end
private
def begin_transaction
return if @db.transaction_active?
@db.execute("BEGIN DEFERRED TRANSACTION")
end
def commit_transaction
return unless @db.transaction_active?
@db.execute("COMMIT")
end
def close_connection(keep_path:)
return if @db.nil?

View File

@ -11,7 +11,7 @@ module Migrations::Database::IntermediateDB
VALUES (?, ?, ?, ?, ?)
SQL
def self.create!(created_at: Time.now, type:, message:, exception: nil, details: nil)
def self.create(created_at: Time.now, type:, message:, exception: nil, details: nil)
::Migrations::Database::IntermediateDB.insert(
SQL,
::Migrations::Database.format_datetime(created_at),

View File

@ -17,8 +17,7 @@ module Migrations::Database::IntermediateDB
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)
SQL
class << self
def create_for_file!(
def self.create_for_file(
path:,
filename: nil,
type: nil,
@ -26,7 +25,7 @@ module Migrations::Database::IntermediateDB
origin: nil,
user_id: nil
)
create!(
create(
id: ::Migrations::ID.hash(path),
filename: filename || File.basename(path),
path:,
@ -37,8 +36,8 @@ module Migrations::Database::IntermediateDB
)
end
def create_for_url!(url:, filename:, type: nil, description: nil, origin: nil, user_id: nil)
create!(
def self.create_for_url(url:, filename:, type: nil, description: nil, origin: nil, user_id: nil)
create(
id: ::Migrations::ID.hash(url),
filename:,
url:,
@ -49,8 +48,15 @@ module Migrations::Database::IntermediateDB
)
end
def create_for_data!(data:, filename:, type: nil, description: nil, origin: nil, user_id: nil)
create!(
def self.create_for_data(
data:,
filename:,
type: nil,
description: nil,
origin: nil,
user_id: nil
)
create(
id: ::Migrations::ID.hash(data),
filename:,
data: ::Migrations::Database.to_blob(data),
@ -61,9 +67,7 @@ module Migrations::Database::IntermediateDB
)
end
private
def create!(
def self.create(
id:,
filename:,
path: nil,
@ -88,5 +92,4 @@ module Migrations::Database::IntermediateDB
)
end
end
end
end

View File

@ -3,7 +3,7 @@
module Migrations::Database
class Migrator
def initialize(db_path)
@db_path = db_path
@db_path = File.expand_path(db_path, ::Migrations.root_path)
@db = nil
end
@ -72,7 +72,7 @@ module Migrations::Database
@db.transaction do
@db.execute(sql)
@db.execute(<<~SQL, path: relative_path, sql_hash: sql_hash)
@db.execute(<<~SQL, path: relative_path, sql_hash:)
INSERT INTO schema_migrations (path, created_at, sql_hash)
VALUES (:path, datetime('now'), :sql_hash)
SQL

View File

@ -71,7 +71,7 @@ module Migrations::Uploader
end
def update_status_queue(row, upload, status)
status_queue << { id: row[:id], upload_id: upload[:id], status: status }
status_queue << { id: row[:id], upload_id: upload[:id], status: }
end
def log_status

View File

@ -7,21 +7,21 @@ require "active_support"
require "active_support/core_ext"
require "zeitwerk"
require_relative "converters"
require_relative "lib/converters"
module Migrations
class NoSettingsFound < StandardError
end
def self.root_path
@root_path ||= File.expand_path("..", __dir__)
@root_path ||= __dir__
end
def self.load_rails_environment(quiet: false)
message = "Loading Rails environment ..."
message = "Loading Rails environment..."
print message if !quiet
rails_root = File.expand_path("../..", __dir__)
rails_root = File.expand_path("..", __dir__)
# rubocop:disable Discourse/NoChdir
Dir.chdir(rails_root) do
begin
@ -48,7 +48,9 @@ module Migrations
{
"cli" => "CLI",
"id" => "ID",
"discourse_db" => "DiscourseDB",
"intermediate_db" => "IntermediateDB",
"mappings_db" => "MappingsDB",
"uploads_db" => "UploadsDB",
},
)
@ -64,7 +66,7 @@ module Migrations
Dir[File.join(converter_path, "**", "*")].each do |subdirectory|
next unless File.directory?(subdirectory)
loader.push_dir(subdirectory, namespace: namespace)
loader.push_dir(subdirectory, namespace:)
end
end

View File

@ -20,7 +20,7 @@ User = Data.define(:id, :name, :email, :created_at)
USER_HASH =
begin
name = SecureRandom.hex(10)
{ id: 1, name: name, email: "#{name}@example.com", created_at: Time.now.utc.iso8601 }
{ id: 1, name:, email: "#{name}@example.com", created_at: Time.now.utc.iso8601 }
end
USER_DATA =

View File

@ -41,7 +41,7 @@ end
def create_users(row_count)
row_count.times.map do |id|
name = SecureRandom.hex(10)
{ id: id, name: name, email: "#{name}@example.com", created_at: Time.now.utc.iso8601 }
{ id:, name:, email: "#{name}@example.com", created_at: Time.now.utc.iso8601 }
end
end

View File

@ -3,7 +3,7 @@
RSpec.describe ::Migrations::Converters::Base::StepTracker do
subject(:tracker) { described_class.new }
before { allow(::Migrations::Database::IntermediateDB::LogEntry).to receive(:create!) }
before { allow(::Migrations::Database::IntermediateDB::LogEntry).to receive(:create) }
describe "#initialize" do
it "starts at the correct values" do
@ -75,7 +75,7 @@ RSpec.describe ::Migrations::Converters::Base::StepTracker do
it "logs an info message" do
tracker.log_info("Info message")
expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create!).with(
expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create).with(
type: ::Migrations::Database::IntermediateDB::LogEntry::INFO,
message: "Info message",
exception: nil,
@ -86,7 +86,7 @@ RSpec.describe ::Migrations::Converters::Base::StepTracker do
it "logs an info message with details" do
tracker.log_info("Info message", details: { key: "value" })
expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create!).with(
expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create).with(
type: ::Migrations::Database::IntermediateDB::LogEntry::INFO,
message: "Info message",
exception: nil,
@ -103,7 +103,7 @@ RSpec.describe ::Migrations::Converters::Base::StepTracker do
tracker.stats.warning_count
}.by(1)
expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create!).with(
expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create).with(
type: ::Migrations::Database::IntermediateDB::LogEntry::WARNING,
message: "Warning message",
exception: nil,
@ -115,13 +115,13 @@ RSpec.describe ::Migrations::Converters::Base::StepTracker do
exception = StandardError.new("Warning exception")
expect {
tracker.log_warning("Warning message", exception: exception, details: { key: "value" })
tracker.log_warning("Warning message", exception:, details: { key: "value" })
}.to change { tracker.stats.warning_count }.by(1)
expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create!).with(
expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create).with(
type: ::Migrations::Database::IntermediateDB::LogEntry::WARNING,
message: "Warning message",
exception: exception,
exception:,
details: {
key: "value",
},
@ -133,7 +133,7 @@ RSpec.describe ::Migrations::Converters::Base::StepTracker do
it "logs an error message and increments error_count" do
expect { tracker.log_error("Error message") }.to change { tracker.stats.error_count }.by(1)
expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create!).with(
expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create).with(
type: ::Migrations::Database::IntermediateDB::LogEntry::ERROR,
message: "Error message",
exception: nil,
@ -145,13 +145,13 @@ RSpec.describe ::Migrations::Converters::Base::StepTracker do
exception = StandardError.new("Error exception")
expect {
tracker.log_error("Error message", exception: exception, details: { key: "value" })
tracker.log_error("Error message", exception:, details: { key: "value" })
}.to change { tracker.stats.error_count }.by(1)
expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create!).with(
expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create).with(
type: ::Migrations::Database::IntermediateDB::LogEntry::ERROR,
message: "Error message",
exception: exception,
exception:,
details: {
key: "value",
},

View File

@ -94,4 +94,25 @@ RSpec.describe ::Migrations::Database::IntermediateDB do
end
end
end
it "checks that manually created entities are tested" do
Dir[
File.join(::Migrations.root_path, "lib", "database", "intermediate_db", "*.rb")
].each do |path|
next if File.read(path).include?("auto-generated")
spec_path =
File.join(
::Migrations.root_path,
"spec",
"lib",
"database",
"intermediate_db",
"#{File.basename(path, ".rb")}_spec.rb",
)
expect(File.exist?(spec_path)).to eq(true), "#{spec_path} is missing"
expect(File.read(spec_path)).to include('it_behaves_like "a database entity"')
end
end
end

View File

@ -9,15 +9,7 @@ RSpec.describe ::Migrations::Database::Migrator do
ignore_errors: false
)
if migrations_directory
migrations_path =
File.join(
::Migrations.root_path,
"spec",
"support",
"fixtures",
"schema",
migrations_directory,
)
migrations_path = File.join(fixture_root, "schema", migrations_directory)
end
temp_path = storage_path = Dir.mktmpdir if storage_path.nil?

View File

@ -3,7 +3,7 @@
# we need to require the rails_helper from core to load the Rails environment
require_relative "../../spec/rails_helper"
require_relative "../lib/migrations"
require_relative "../migrations"
::Migrations.configure_zeitwerk
::Migrations.enable_i18n

View File

@ -5,3 +5,7 @@ def reset_memoization(instance, *variables)
instance.remove_instance_variable(var) if instance.instance_variable_defined?(var)
end
end
def fixture_root
@fixture_root ||= File.join(::Migrations.root_path, "spec", "support", "fixtures")
end

View File

@ -5,7 +5,7 @@ RSpec.shared_examples "a database entity" do
expect(subject).to have_constant(:SQL)
end
it "responds to .create!" do
expect(subject).to respond_to(:create!)
it "responds to .create" do
expect(subject).to respond_to(:create)
end
end