Migrar configurações deve levar GlobalSettings em consideração

Bug
1

Tenho visto algumas migrações de configurações recentemente (a mais recente sendo a remoção de automatic_backups_enabled) onde a migração usa apenas valores do banco de dados para calcular um novo valor. Isso ignora quaisquer configurações feitas em discourse.conf via app.yml.

Código:

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

que ignora automatic_backups_enabled = false em discourse.conf e, como resultado, não mantém os backups desativados quando essa configuração está presente.

Este navio específico já partiu, mas pode ser bom ter em mente que esse padrão causa problemas com configurações que são substituídas globalmente.

4 curtidas
2

Acho que neste caso, no futuro, devemos observar DISCOURSE_... no Env durante essas migrações.

@ted Acho que vale a pena acompanhar retroativamente, pois estabelecerá o padrão daqui para frente.

Não tenho certeza se GlobalSetting é carregado ou se é algo em que queremos confiar, mas dar uma olhada em

ENV['DISCOURSE_AUTOMATIC_BACKUPS_ENABLED'] e fazê-lo prevalecer sobre a configuração no banco de dados é provavelmente sensato.

@tgxworld / @ted talvez devêssemos adicionar um “helper de migração” para isso?

Por exemplo:

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

Sim, GlobalSetting é carregado durante as migrações :+1:

Eu também não acho que você queira confiar em uma variável de ambiente em tempo de compilação? Isso torna difícil separar a etapa da variável de ambiente para discourse.conf e a implantação real do contêiner. Em meu outro mundo (PHP/Laravel), isso é visto como um anti-padrão.

1 curtida