Commit 7b4d27b0 authored by Kassio Borges's avatar Kassio Borges

Remove feature flag

Remove github_review_importer_query_only_unimported_merge_requests
feature flag as the new branch have not caused any problems in
production.

Changelog: changed
MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65473
parent 4441b7c9
---
name: github_review_importer_query_only_unimported_merge_requests
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/62036
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/332982
milestone: '14.0'
type: development
group: group::import
default_enabled: true
......@@ -37,43 +37,6 @@ module Gitlab
review.id
end
def each_object_to_import(&block)
if use_github_review_importer_query_only_unimported_merge_requests?
each_merge_request_to_import(&block)
else
each_merge_request_skipping_imported(&block)
end
end
private
attr_reader :merge_requests_already_imported_cache_key
# https://gitlab.com/gitlab-org/gitlab/-/merge_requests/62036#note_587181108
def use_github_review_importer_query_only_unimported_merge_requests?
Feature.enabled?(
:github_review_importer_query_only_unimported_merge_requests,
default_enabled: :yaml
)
end
def each_merge_request_skipping_imported
project.merge_requests.find_each do |merge_request|
next if already_imported?(merge_request)
Gitlab::GithubImport::ObjectCounter.increment(project, object_type, :fetched)
client
.pull_request_reviews(project.import_source, merge_request.iid)
.each do |review|
review.merge_request_id = merge_request.id
yield(review)
end
mark_as_imported(merge_request)
end
end
# The worker can be interrupted, by rate limit for instance,
# in different situations. To avoid requesting already imported data,
# if the worker is interrupted:
......@@ -82,7 +45,7 @@ module Gitlab
# - before importing all merge requests reviews
# Merge requests that had all the reviews imported are cached with
# `mark_merge_request_reviews_imported`
def each_merge_request_to_import
def each_object_to_import(&block)
each_review_page do |page, merge_request|
page.objects.each do |review|
next if already_imported?(review)
......@@ -97,6 +60,10 @@ module Gitlab
end
end
private
attr_reader :merge_requests_already_imported_cache_key
def each_review_page
merge_requests_to_import.find_each do |merge_request|
# The page counter needs to be scoped by merge request to avoid skipping
......
......@@ -27,100 +27,62 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestsReviewsImporter do
end
describe '#each_object_to_import', :clean_gitlab_redis_cache do
context 'when github_review_importer_query_only_unimported_merge_requests is enabled' do
before do
stub_feature_flags(github_review_importer_query_only_unimported_merge_requests: true)
end
let(:merge_request) do
create(
:merged_merge_request,
iid: 999,
source_project: project,
target_project: project
)
end
let(:review) { double(id: 1) }
it 'fetches the pull requests reviews data' do
page = double(objects: [review], number: 1)
expect(review)
.to receive(:merge_request_id=)
.with(merge_request.id)
expect(client)
.to receive(:each_page)
.exactly(:once) # ensure to be cached on the second call
.with(:pull_request_reviews, 'github/repo', merge_request.iid, page: 1)
.and_yield(page)
let(:merge_request) do
create(
:merged_merge_request,
iid: 999,
source_project: project,
target_project: project
)
end
expect { |b| subject.each_object_to_import(&b) }
.to yield_with_args(review)
let(:review) { double(id: 1) }
subject.each_object_to_import {}
end
it 'fetches the pull requests reviews data' do
page = double(objects: [review], number: 1)
it 'skips cached pages' do
Gitlab::GithubImport::PageCounter
.new(project, "merge_request/#{merge_request.id}/pull_request_reviews")
.set(2)
expect(review)
.to receive(:merge_request_id=)
.with(merge_request.id)
expect(review).not_to receive(:merge_request_id=)
expect(client)
.to receive(:each_page)
.exactly(:once) # ensure to be cached on the second call
.with(:pull_request_reviews, 'github/repo', merge_request.iid, page: 1)
.and_yield(page)
expect(client)
.to receive(:each_page)
.exactly(:once) # ensure to be cached on the second call
.with(:pull_request_reviews, 'github/repo', merge_request.iid, page: 2)
expect { |b| subject.each_object_to_import(&b) }
.to yield_with_args(review)
subject.each_object_to_import {}
end
subject.each_object_to_import {}
end
it 'skips cached merge requests' do
Gitlab::Cache::Import::Caching.set_add(
"github-importer/merge_request/already-imported/#{project.id}",
merge_request.id
)
it 'skips cached pages' do
Gitlab::GithubImport::PageCounter
.new(project, "merge_request/#{merge_request.id}/pull_request_reviews")
.set(2)
expect(review).not_to receive(:merge_request_id=)
expect(review).not_to receive(:merge_request_id=)
expect(client).not_to receive(:each_page)
expect(client)
.to receive(:each_page)
.exactly(:once) # ensure to be cached on the second call
.with(:pull_request_reviews, 'github/repo', merge_request.iid, page: 2)
subject.each_object_to_import {}
end
subject.each_object_to_import {}
end
context 'when github_review_importer_query_only_unimported_merge_requests is disabled' do
before do
stub_feature_flags(github_review_importer_query_only_unimported_merge_requests: false)
end
it 'fetchs the merged pull requests data' do
merge_request = create(
:merged_merge_request,
iid: 999,
source_project: project,
target_project: project
)
review = double
expect(review)
.to receive(:merge_request_id=)
.with(merge_request.id)
it 'skips cached merge requests' do
Gitlab::Cache::Import::Caching.set_add(
"github-importer/merge_request/already-imported/#{project.id}",
merge_request.id
)
allow(client)
.to receive(:pull_request_reviews)
.exactly(:once) # ensure to be cached on the second call
.with('github/repo', merge_request.iid)
.and_return([review])
expect(review).not_to receive(:merge_request_id=)
expect { |b| subject.each_object_to_import(&b) }
.to yield_with_args(review)
expect(client).not_to receive(:each_page)
subject.each_object_to_import {}
end
subject.each_object_to_import {}
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment