Миграция настроек должна учитывать GlobalSettings

Внести вклад Ошибка
1

Я недавно заметил несколько миграций настроек (последняя из них — удаление automatic_backups_enabled), в которых миграция использует только значения из базы данных для вычисления нового значения. Это игнорирует любые настройки, заданные в discourse.conf через app.yml.

Код:

INSERT INTO site_settings (name, data_type, value, created_at, updated_at)
      SELECT 'backup_frequency', 3, NULL, 'NOW()', 'NOW()'
      WHERE EXISTS (
        SELECT 1
        FROM site_settings
        WHERE name = 'automatic_backups_enabled'
        AND VALUE = 'f'
        LIMIT 1
      )
      ON CONFLICT (name) DO UPDATE SET value = NULL, updated_at = 'NOW()';

Этот код игнорирует automatic_backups_enabled = false в discourse.conf, и в результате резервное копирование не остаётся отключённым, даже если такая настройка присутствует.

Конкретный случай уже упущен, но стоит помнить, что такой подход создаёт проблемы с настройками, которые переопределяются глобально.

4 лайка
2

Полагаю, в данном случае в будущем нам стоит обращать внимание на DISCOURSE_... в Env во время этих миграций.

@ted, думаю, стоит отработать это ретроспективно, так как это задаст паттерн на будущее.

Не уверен, загружается ли GlobalSetting и стоит ли на него опираться, но, на мой взгляд, разумно проверить

ENV['DISCOURSE_AUTOMATIC_BACKUPS_ENABLED'] и разрешить ему иметь приоритет над настройкой в базе данных.

@tgxworld / @ted, может, стоит добавить для этого «помощника миграции»?

Например:

MigrationHelper.get_setting('abc')
MigrationHelper.set_setting('abc', '123')
3 лайка
4

Да, GlobalSetting загружается во время миграций :+1:

И, думаю, вам тоже не стоит полагаться на переменную окружения времени сборки? Это усложняет разделение шага установки переменной окружения в discourse.conf и фактического развёртывания контейнера. В моей другой сфере (PHP/Laravel) это считается антипаттерном.

1 лайк