La migrazione delle impostazioni dovrebbe tenere conto di GlobalSettings

Bug
1

Ho visto alcune migrazioni di impostazioni di recente (l’ultima è stata la rimozione di automatic_backups_enabled) in cui la migrazione utilizza solo valori del database per calcolare un nuovo valore. Ciò ignora qualsiasi impostazione effettuata in discourse.conf tramite app.yml.

Codice:

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()';

che ignora automatic_backups_enabled = false in discourse.conf e, di conseguenza, non mantiene i backup disabilitati quando tale impostazione è presente.

Questa specifica nave è salpata, ma potrebbe essere utile tenere presente che questo schema causa problemi con le impostazioni che vengono sovrascritte a livello globale.

4 Mi Piace
2

In questo caso, in futuro dovremmo esaminare DISCOURSE_... in Env durante queste migrazioni.

@ted Penso che valga la pena dare seguito retroattivamente perché stabilirà il modello per il futuro.

Non sono sicuro se GlobalSetting sia caricato o se sia qualcosa su cui vogliamo fare affidamento, ma dando un’occhiata a

ENV['DISCOURSE_AUTOMATIC_BACKUPS_ENABLED'] e facendolo prevalere sull’impostazione nel DB è probabilmente saggio.

@tgxworld / @ted forse dovremmo aggiungere un “migration helper” per questo?

Ad esempio:

MigrationHelper.get_setting('abc')
MigrationHelper.set_setting('abc', '123')
3 Mi Piace
4

Sì, GlobalSetting viene caricato durante le migrazioni :+1:

Non credo che tu voglia fare affidamento nemmeno su una variabile d’ambiente in fase di compilazione? Rende difficile separare il passaggio da variabile d’ambiente a discourse.conf e l’effettiva distribuzione del container. Nel mio altro mondo (PHP/Laravel) questo è visto come un anti-pattern.

1 Mi Piace