Draft: Refactor Gitlab::CurrentSettings so it has less ApplicationSetting-specific code Part I
What does this MR do and why?
This is part One of two MR's related to a small refactoring of GitLab::CurrentSetting
class. This class has too many responsibilities:
- A single entry point for Application Settings
- Taking care of available settings in case the database or Redis is not available, taking care of settings in test context
In the near future, we want this class to also read from Organization Settings, so we want to remove the ApplicationSetting-specific code.
This will happen in two MR's:
- This MR:
- Introduce
Gitlab::ApplicationSettingFetcher
and port the part that is using the cached (Redis) version of ApplicationSettings
- Introduce
- Next MR:
- port the code that handles uncached version of ApplicationSettings (missing database connection, missing Redis, test environment)
The code change is behind a feature flag considering the risk
How to set up and validate locally
- All the tests should pass (using label pipeline:run-all-rspec )
- With feature flag
refactor_current_settings_caching
enabled and also disabled:- Edge case: AppliationSetting can be used while the database is not available. This is supported only for rake tasks
-
gdk stop postgresql && bundle exec rake gitlab:shell:setup
(it can be aborted before it does something), this should not raiseActiveRecord::ConnectionNotEstablished: could not connect to server
-
- Edge case: AppliationSetting can be used while the database is not available. This is supported only for rake tasks
-
gdk start
and then test the web application locally
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #417763 (closed)
Edited by Rutger Wessels