From cd22da3ebda969779c47f1652f3876daa5edd7f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 25 Jan 2016 20:00:59 +0100 Subject: Fix preventing migration from crashing in very specific cases See https://gitlab.com/gitlab-org/gitlab-ce/issues/12606 for details --- lib/gitlab/current_settings.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index ea054255820..429f1f9bb56 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -39,6 +39,13 @@ module Gitlab end use_db && ActiveRecord::Base.connection.active? && + # The following condition is important: if a migrations adds a + # column to the application_settings table and a validation in + # the ApplicationSetting uses this new column we might end-up in + # a vicious circle where migration crash before being done. + # See https://gitlab.com/gitlab-org/gitlab-ce/issues/12606 for + # a thorough explanation. + !ActiveRecord::Migrator.needs_migration? && ActiveRecord::Base.connection.table_exists?('application_settings') rescue ActiveRecord::NoDatabaseError -- cgit v1.2.1 From e6f3fe5d3b16809634dd655b3d265c683b538b12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 26 Jan 2016 15:11:15 +0100 Subject: Ensure rake tasks that don't need a DB connection can be run without one When using ActiveRecord::Base.connection.active? without a DB connection, we get a "PG::ConnectionBad: could not connect to server" error because ActiveRecord::Base.connection doesn't exist. By using ActiveRecord::Base.connected? we ensure we don't get this error if the connection is missing, which is the all point of the Gitlab::CurrentSettings#connect_to_db? method! --- CHANGELOG | 1 + lib/gitlab/current_settings.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index b3b4aa380d5..97606659f35 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.5.0 (unreleased) + - Ensure rake tasks that don't need a DB connection can be run without one - Add "visibility" flag to GET /projects api endpoint - Ignore binary files in code search to prevent Error 500 (Stan Hu) - Upgrade gitlab_git to 7.2.23 to fix commit message mentions in first branch push diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index 429f1f9bb56..19b7427256c 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -38,7 +38,7 @@ module Gitlab true end - use_db && ActiveRecord::Base.connection.active? && + use_db && ActiveRecord::Base.connected? && # The following condition is important: if a migrations adds a # column to the application_settings table and a validation in # the ApplicationSetting uses this new column we might end-up in -- cgit v1.2.1 From 869b4d7c6ad5c025f310eeecce7293bf5bbc4926 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 28 Jan 2016 10:14:21 +0100 Subject: Only create the defaults ApplicationSetting when DB is fully migrated Return a fake application settings OpenStruct when this is not the case. Also, use ActiveRecord::Base.connection_pool.active_connection? instead of ActiveRecord::Base.connection.active? to avoid driver exception. --- lib/gitlab/current_settings.rb | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index 19b7427256c..e66e7768f2b 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -4,11 +4,14 @@ module Gitlab key = :current_application_settings RequestStore.store[key] ||= begin + settings = nil + if connect_to_db? - ApplicationSetting.current || ApplicationSetting.create_from_defaults - else - fake_application_settings + settings = ApplicationSetting.current + settings ||= ApplicationSetting.create_from_defaults unless ActiveRecord::Migrator.needs_migration? end + + settings || fake_application_settings end end @@ -18,35 +21,29 @@ module Gitlab default_branch_protection: Settings.gitlab['default_branch_protection'], signup_enabled: Settings.gitlab['signup_enabled'], signin_enabled: Settings.gitlab['signin_enabled'], + twitter_sharing_enabled: Settings.gitlab['twitter_sharing_enabled'], gravatar_enabled: Settings.gravatar['enabled'], sign_in_text: Settings.extra['sign_in_text'], restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels'], max_attachment_size: Settings.gitlab['max_attachment_size'], session_expire_delay: Settings.gitlab['session_expire_delay'], - import_sources: Settings.gitlab['import_sources'], + default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'], + default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'], + restricted_signup_domains: Settings.gitlab['restricted_signup_domains'], + import_sources: ['github','bitbucket','gitlab','gitorious','google_code','fogbugz','git'], shared_runners_enabled: Settings.gitlab_ci['shared_runners_enabled'], max_artifacts_size: Settings.artifacts['max_size'], + require_two_factor_authentication: false, + two_factor_grace_period: 48 ) end private def connect_to_db? - use_db = if ENV['USE_DB'] == "false" - false - else - true - end - - use_db && ActiveRecord::Base.connected? && - # The following condition is important: if a migrations adds a - # column to the application_settings table and a validation in - # the ApplicationSetting uses this new column we might end-up in - # a vicious circle where migration crash before being done. - # See https://gitlab.com/gitlab-org/gitlab-ce/issues/12606 for - # a thorough explanation. - !ActiveRecord::Migrator.needs_migration? && - ActiveRecord::Base.connection.table_exists?('application_settings') + ENV['USE_DB'] != 'false' && + ActiveRecord::Base.connection_pool.active_connection? && + ActiveRecord::Base.connection.table_exists?('application_settings') rescue ActiveRecord::NoDatabaseError false -- cgit v1.2.1 From 55ab92c00bb79a951dea477d59e440c80dc96f91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 28 Jan 2016 12:23:37 +0100 Subject: Use ActiveRecord::Base.connection.active? and rescue any exception in connect_to_db? This ensures that rake tasks that don't need a DB connection can be run without one. --- lib/gitlab/current_settings.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index e66e7768f2b..a6b2f14521c 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -41,8 +41,11 @@ module Gitlab private def connect_to_db? + # When the DBMS is not available, an exception (e.g. PG::ConnectionBad) is raised + active_db_connection = ActiveRecord::Base.connection.active? rescue false + ENV['USE_DB'] != 'false' && - ActiveRecord::Base.connection_pool.active_connection? && + active_db_connection && ActiveRecord::Base.connection.table_exists?('application_settings') rescue ActiveRecord::NoDatabaseError -- cgit v1.2.1