From 735e7717e7656cd449affd989fc16f2228f7dd00 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Sun, 24 May 2020 12:08:20 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .rubocop.yml | 2 - app/models/ci/build.rb | 2 +- app/models/repository.rb | 6 +-- app/models/ssh_host_key.rb | 2 +- .../hashed_storage/base_attachment_service.rb | 2 +- changelogs/unreleased/ee-app-services-1.yml | 5 ++ .../unreleased/leaky-constant-fix-36.yml | 5 ++ .../unreleased/leaky-constant-fix-9.yml | 5 ++ changelogs/unreleased/rails-logger-cop-15.yml | 5 ++ changelogs/unreleased/rails-logger-cop-4.yml | 5 ++ changelogs/unreleased/rails-logger-cop-5.yml | 5 ++ lib/feature.rb | 4 +- spec/lib/gitlab/git/diff_collection_spec.rb | 46 ++++++++++--------- spec/models/ci/build_spec.rb | 4 +- spec/models/repository_spec.rb | 2 +- .../check_uninstall_progress_service_spec.rb | 4 +- 16 files changed, 67 insertions(+), 37 deletions(-) create mode 100644 changelogs/unreleased/ee-app-services-1.yml create mode 100644 changelogs/unreleased/leaky-constant-fix-36.yml create mode 100644 changelogs/unreleased/leaky-constant-fix-9.yml create mode 100644 changelogs/unreleased/rails-logger-cop-15.yml create mode 100644 changelogs/unreleased/rails-logger-cop-4.yml create mode 100644 changelogs/unreleased/rails-logger-cop-5.yml diff --git a/.rubocop.yml b/.rubocop.yml index 4dad1fb7a18..037e8c4e00f 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -351,7 +351,6 @@ RSpec/LeakyConstantDeclaration: - 'spec/db/schema_spec.rb' - 'spec/lib/feature_spec.rb' - 'spec/lib/gitlab/config/entry/simplifiable_spec.rb' - - 'spec/lib/gitlab/git/diff_collection_spec.rb' - 'spec/lib/gitlab/import_export/import_test_coverage_spec.rb' - 'spec/lib/gitlab/quick_actions/dsl_spec.rb' - 'spec/lib/marginalia_spec.rb' @@ -363,7 +362,6 @@ RSpec/LeakyConstantDeclaration: - 'spec/models/concerns/triggerable_hooks_spec.rb' - 'spec/models/repository_spec.rb' - 'spec/services/clusters/applications/check_installation_progress_service_spec.rb' - - 'spec/services/clusters/applications/check_uninstall_progress_service_spec.rb' - 'spec/support/shared_examples/quick_actions/issuable/issuable_quick_actions_shared_examples.rb' RSpec/EmptyLineAfterHook: diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index d6f6515ed23..2b67bc543b5 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -352,7 +352,7 @@ module Ci begin Ci::Build.retry(build, build.user) rescue Gitlab::Access::AccessDeniedError => ex - Rails.logger.error "Unable to auto-retry job #{build.id}: #{ex}" # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.error "Unable to auto-retry job #{build.id}: #{ex}" end end end diff --git a/app/models/repository.rb b/app/models/repository.rb index 16b4fdabf48..911cfc7db38 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -950,7 +950,6 @@ class Repository async_remove_remote(remote_name) if tmp_remote_name end - # rubocop:disable Gitlab/RailsLogger def async_remove_remote(remote_name) return unless remote_name return unless project @@ -958,14 +957,13 @@ class Repository job_id = RepositoryRemoveRemoteWorker.perform_async(project.id, remote_name) if job_id - Rails.logger.info("Remove remote job scheduled for #{project.id} with remote name: #{remote_name} job ID #{job_id}.") + Gitlab::AppLogger.info("Remove remote job scheduled for #{project.id} with remote name: #{remote_name} job ID #{job_id}.") else - Rails.logger.info("Remove remote job failed to create for #{project.id} with remote name #{remote_name}.") + Gitlab::AppLogger.info("Remove remote job failed to create for #{project.id} with remote name #{remote_name}.") end job_id end - # rubocop:enable Gitlab/RailsLogger def fetch_source_branch!(source_repository, source_branch, local_ref) raw_repository.fetch_source_branch!(source_repository.raw_repository, source_branch, local_ref) diff --git a/app/models/ssh_host_key.rb b/app/models/ssh_host_key.rb index 72690ad7d04..7e34988c7a0 100644 --- a/app/models/ssh_host_key.rb +++ b/app/models/ssh_host_key.rb @@ -107,7 +107,7 @@ class SshHostKey if status.success? && !errors.present? { known_hosts: known_hosts } else - Rails.logger.debug("Failed to detect SSH host keys for #{id}: #{errors}") # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.debug("Failed to detect SSH host keys for #{id}: #{errors}") { error: 'Failed to detect SSH host keys' } end diff --git a/app/services/projects/hashed_storage/base_attachment_service.rb b/app/services/projects/hashed_storage/base_attachment_service.rb index a2a7895ba17..d61a2af6c1c 100644 --- a/app/services/projects/hashed_storage/base_attachment_service.rb +++ b/app/services/projects/hashed_storage/base_attachment_service.rb @@ -19,7 +19,7 @@ module Projects def initialize(project:, old_disk_path:, logger: nil) @project = project @old_disk_path = old_disk_path - @logger = logger || Rails.logger # rubocop:disable Gitlab/RailsLogger + @logger = logger || Gitlab::AppLogger end # Return whether this operation was skipped or not diff --git a/changelogs/unreleased/ee-app-services-1.yml b/changelogs/unreleased/ee-app-services-1.yml new file mode 100644 index 00000000000..bbd916a9d2c --- /dev/null +++ b/changelogs/unreleased/ee-app-services-1.yml @@ -0,0 +1,5 @@ +--- +title: Move prepend to last in ee-app-services +merge_request: 31838 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/leaky-constant-fix-36.yml b/changelogs/unreleased/leaky-constant-fix-36.yml new file mode 100644 index 00000000000..1c487a8329e --- /dev/null +++ b/changelogs/unreleased/leaky-constant-fix-36.yml @@ -0,0 +1,5 @@ +--- +title: Fix leaky constant issue in diff collection spec +merge_request: 32163 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/leaky-constant-fix-9.yml b/changelogs/unreleased/leaky-constant-fix-9.yml new file mode 100644 index 00000000000..e361c58c746 --- /dev/null +++ b/changelogs/unreleased/leaky-constant-fix-9.yml @@ -0,0 +1,5 @@ +--- +title: Fix leaky constant issue in uninstall progress service check +merge_request: 32036 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rails-logger-cop-15.yml b/changelogs/unreleased/rails-logger-cop-15.yml new file mode 100644 index 00000000000..1c9ccafe5cb --- /dev/null +++ b/changelogs/unreleased/rails-logger-cop-15.yml @@ -0,0 +1,5 @@ +--- +title: Use applogger in base attachment service +merge_request: 32201 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rails-logger-cop-4.yml b/changelogs/unreleased/rails-logger-cop-4.yml new file mode 100644 index 00000000000..a7f76711de7 --- /dev/null +++ b/changelogs/unreleased/rails-logger-cop-4.yml @@ -0,0 +1,5 @@ +--- +title: Use applogger in repository model +merge_request: 32185 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rails-logger-cop-5.yml b/changelogs/unreleased/rails-logger-cop-5.yml new file mode 100644 index 00000000000..4e758bde275 --- /dev/null +++ b/changelogs/unreleased/rails-logger-cop-5.yml @@ -0,0 +1,5 @@ +--- +title: Use applogger in build and ssh host key +merge_request: 32187 +author: Rajendra Kadam +type: fixed diff --git a/lib/feature.rb b/lib/feature.rb index dc7e8da8f35..d1c9ff047dc 100644 --- a/lib/feature.rb +++ b/lib/feature.rb @@ -4,8 +4,6 @@ require 'flipper/adapters/active_record' require 'flipper/adapters/active_support_cache_store' class Feature - prepend_if_ee('EE::Feature') # rubocop: disable Cop/InjectEnterpriseEditionModule - # Classes to override flipper table names class FlipperFeature < Flipper::Adapters::ActiveRecord::Feature # Using `self.table_name` won't work. ActiveRecord bug? @@ -186,3 +184,5 @@ class Feature end end end + +Feature.prepend_if_ee('EE::Feature') diff --git a/spec/lib/gitlab/git/diff_collection_spec.rb b/spec/lib/gitlab/git/diff_collection_spec.rb index 0d19d35bc52..6aa4f884d20 100644 --- a/spec/lib/gitlab/git/diff_collection_spec.rb +++ b/spec/lib/gitlab/git/diff_collection_spec.rb @@ -3,6 +3,31 @@ require 'spec_helper' describe Gitlab::Git::DiffCollection, :seed_helper do + before do + stub_const('MutatingConstantIterator', Class.new) + + MutatingConstantIterator.class_eval do + include Enumerable + + def initialize(count, value) + @count = count + @value = value + end + + def each + return enum_for(:each) unless block_given? + + loop do + break if @count.zero? + + # It is critical to decrement before yielding. We may never reach the lines after 'yield'. + @count -= 1 + yield @value + end + end + end + end + subject do Gitlab::Git::DiffCollection.new( iterator, @@ -659,25 +684,4 @@ describe Gitlab::Git::DiffCollection, :seed_helper do def fake_diff(line_length, line_count) { 'diff' => "#{'a' * line_length}\n" * line_count } end - - class MutatingConstantIterator - include Enumerable - - def initialize(count, value) - @count = count - @value = value - end - - def each - return enum_for(:each) unless block_given? - - loop do - break if @count.zero? - - # It is critical to decrement before yielding. We may never reach the lines after 'yield'. - @count -= 1 - yield @value - end - end - end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 2065e132de7..ddf12f68160 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -3612,7 +3612,7 @@ describe Ci::Build do .to receive(:execute) .with(subject) .and_raise(Gitlab::Access::AccessDeniedError) - allow(Rails.logger).to receive(:error) + allow(Gitlab::AppLogger).to receive(:error) end it 'handles raised exception' do @@ -3622,7 +3622,7 @@ describe Ci::Build do it 'logs the error' do subject.drop! - expect(Rails.logger) + expect(Gitlab::AppLogger) .to have_received(:error) .with(a_string_matching("Unable to auto-retry job #{subject.id}")) end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index a616a63e724..c4b91e41c02 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -974,7 +974,7 @@ describe Repository do end it 'returns nil' do - expect(Rails.logger).to receive(:info).with("Remove remote job failed to create for #{project.id} with remote name joe.") + expect(Gitlab::AppLogger).to receive(:info).with("Remove remote job failed to create for #{project.id} with remote name joe.") expect(repository.async_remove_remote('joe')).to be_nil end diff --git a/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb b/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb index ffb658330d3..9dede1947f8 100644 --- a/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb +++ b/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Clusters::Applications::CheckUninstallProgressService do - RESCHEDULE_PHASES = Gitlab::Kubernetes::Pod::PHASES - [Gitlab::Kubernetes::Pod::SUCCEEDED, Gitlab::Kubernetes::Pod::FAILED].freeze + reschedule_phases = Gitlab::Kubernetes::Pod::PHASES - [Gitlab::Kubernetes::Pod::SUCCEEDED, Gitlab::Kubernetes::Pod::FAILED].freeze let(:application) { create(:clusters_applications_prometheus, :uninstalling) } let(:service) { described_class.new(application) } @@ -42,7 +42,7 @@ describe Clusters::Applications::CheckUninstallProgressService do end context 'when application is uninstalling' do - RESCHEDULE_PHASES.each { |phase| it_behaves_like 'a not yet terminated installation', phase } + reschedule_phases.each { |phase| it_behaves_like 'a not yet terminated installation', phase } context 'when installation POD succeeded' do let(:phase) { Gitlab::Kubernetes::Pod::SUCCEEDED }