Project

General

Profile

Actions

Feature #36933

open

Halt Redmine boot entirely instead of showing a warning in admin/info when there are pending migrations

Added by Mischa The Evil over 3 years ago. Updated 5 days ago.

Status:
Needs feedback
Priority:
Normal
Category:
Administration
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:

Description

Starting with Redmine 5.0.0 admin/info shows a warning when there are pending migrations (#35562). I think it makes no sense to let Redmine boot at all if there are pending migrations. It might even lead to data corruption in the DB.
It might be better to raise an exception and halt the boot process altogether with an initializer (e.g. /config/initializers/05-check_pending_migrations.rb) containing:

if Rails.env.production?
  ActiveRecord::Migration.check_pending!
end

What do you think?


Files


Related issues

Related to Redmine - Feature #35562: Show warning in admin/info when there are pending migrationsClosedGo MAEDA

Actions
Actions #1

Updated by Mischa The Evil almost 2 years ago

  • Status changed from New to Needs feedback
  • Target version set to 6.0.0

Given the confirmed potential implications of the issue, I'd would like to propose/discuss this for the upcoming (major) release.

Actions #2

Updated by Marius BĂLTEANU almost 2 years ago

I'm not sure if we should entirely halt the boot or we should just throw some warnings in the console.

Actions #3

Updated by Mischa The Evil almost 2 years ago

Marius BALTEANU wrote in #note-2:

I'm not sure if we should entirely halt the boot or we should just throw some warnings in the console.

Let me put it in another way: why should we let Redmine boot into a (production) state that we know upfront, sooner or later will result in errors and/or potential data corruption? Am I missing something? Is there some use-case?

FWIW: I do agree that if we were to let Redmine halt the boot process, we should also provide a clear and meaningful error message to the user.

Actions #4

Updated by Marius BĂLTEANU 11 months ago

  • Target version changed from 6.0.0 to 6.1.0
Actions #5

Updated by Holger Just 11 months ago

Outright refusing to start entirely with pending migrations may be a bad idea as it may prevent people from running rake tasks which require the environment to load or even just to start a rails console to fix issues with a pending/failed migration.

Instead, if this is in fact desired, we could set config.active_record.migration_error = :page_load in the production environment (it's enabled by default in development). This will insert the ActiveRecord::Migration::CheckPending rack middleware which will raise a (handled) error if there are any pending migrations when loading a page. Maybe we could even make that configureable in configuration.yml with the setting enabled by default.

It still doesn't feel 100% right, altough I can't really figure out why that is :) But I do think it is actually a good idea to nudge admins to actually run the migrations. In the end, we might try the middleware approach if that works for "common" upgrades. If there are some missing edge-cases, we may address them as they come up.

Actions #6

Updated by Marius BĂLTEANU 11 days ago

  • Related to Feature #35562: Show warning in admin/info when there are pending migrations added
Actions #7

Updated by Marius BĂLTEANU 11 days ago

In #35562 we added an warning message in /admin/info when pending migration exists, we can do the same at boot level by throwing an warning in the console:

root@423fa770a0e3:/work# bin/rails s -b 0.0.0.0
=> Booting Puma
=> Rails 7.2.2.2 application starting in production 
=> Run `bin/rails server --help` for more startup options

============================================================
[WARNING] You have pending migrations. Run `bundle exec rake db:migrate` to apply them.
============================================================

Puma starting in single mode...
* Puma version: 6.4.3 (ruby 3.2.9-p265) ("The Eagle of Durango")
*  Min threads: 0
*  Max threads: 5
*  Environment: production
*          PID: 2271
* Listening on http://0.0.0.0:3000
Use Ctrl-C to stop

This new check happens only in production mode and when the context is a Rails server or console.

Holger Just, Mischa The Evil, what do you thing about this implementation?

Actions #8

Updated by Marius BĂLTEANU 11 days ago

  • File deleted (0001-In-production-mode-check-for-pending-migration-on-bo.patch)
Actions #10

Updated by Marius BĂLTEANU 5 days ago

  • Target version changed from 6.1.0 to Candidate for next major release
Actions

Also available in: Atom PDF