설정 마이그레이션 시 GlobalSettings를 고려해야 합니다

기여 버그
1

최근 몇 가지 설정 마이그레이션을 확인했는데(가장 최근의 예는 automatic_backups_enabled 제거) 마이그레이션이 새 값을 계산하기 위해 데이터베이스 값만을 사용하고 있습니다. 이는 app.yml를 통해 discourse.conf에서 설정된 값을 무시하게 됩니다.

코드:

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

이 코드는 discourse.conf의 automatic_backups_enabled = false를 무시하므로, 해당 설정이 존재하는 경우에도 백업이 비활성화된 상태로 유지되지 않습니다.

이 특정 사례는 이미 지나갔지만, 전역적으로 오버라이드된 설정에서 이 패턴이 문제를 일으킬 수 있다는 점을 염두에 두는 것이 좋을 것입니다.

4개의 좋아요
2

이번 경우에는 앞으로 이러한 마이그레이션 시 Env에서 DISCOURSE_...를 살펴봐야 할 것 같습니다.

@ted 앞으로의 패턴을 정립할 수 있으므로 사후적으로 후속 조치를 취하는 것이 가치가 있다고 생각합니다.

GlobalSetting이 로드되는지, 아니면 우리가 의존해야 하는 것인지는 확실하지 않지만, ENV['DISCOURSE_AUTOMATIC_BACKUPS_ENABLED']를 확인하고 DB의 설정보다 이 값이 우선하도록 하는 것이 현명할 것입니다.

@tgxworld / @ted 이걸 위해 "마이그레이션 헬퍼"를 추가하는 것이 어떨까요?

예:

MigrationHelper.get_setting('abc')
MigrationHelper.set_setting('abc', '123')
3개의 좋아요
4

네, GlobalSetting은 마이그레이션 중에 로드됩니다 :+1:

빌드 시간 ENV 변수에 의존하는 것도 원치 않을 것 같습니다. 이렇게 하면 ENV 변수를 discourse.conf 단계와 실제 컨테이너 배포로 분리하기가 어려워집니다. 제 다른 세계(PHP/Laravel)에서는 이것이 안티 패턴으로 간주됩니다.

1개의 좋아요