Commit 8e51439e authored by Shinya Maeda's avatar Shinya Maeda

Drop legacy artifacts usage

Legacy artifacts have been correctly migrated to new place -
ci_job_artifacts. Now it's time to remove the related code, but before
that we should ensure it doesn't break anything by using feature flag.
parent 74ace2a4
...@@ -83,8 +83,13 @@ module Ci ...@@ -83,8 +83,13 @@ module Ci
scope :unstarted, ->() { where(runner_id: nil) } scope :unstarted, ->() { where(runner_id: nil) }
scope :ignore_failures, ->() { where(allow_failure: false) } scope :ignore_failures, ->() { where(allow_failure: false) }
scope :with_artifacts_archive, ->() do scope :with_artifacts_archive, ->() do
where('(artifacts_file IS NOT NULL AND artifacts_file <> ?) OR EXISTS (?)', if Feature.enabled?(:ci_enable_legacy_artifacts)
'', Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id').archive) where('(artifacts_file IS NOT NULL AND artifacts_file <> ?) OR EXISTS (?)',
'', Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id').archive)
else
where('EXISTS (?)',
Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id').archive)
end
end end
scope :with_existing_job_artifacts, ->(query) do scope :with_existing_job_artifacts, ->(query) do
...@@ -135,6 +140,8 @@ module Ci ...@@ -135,6 +140,8 @@ module Ci
where("EXISTS (?)", matcher) where("EXISTS (?)", matcher)
end end
##
# TODO: Remove these mounters when we remove :ci_enable_legacy_artifacts feature flag
mount_uploader :legacy_artifacts_file, LegacyArtifactUploader, mount_on: :artifacts_file mount_uploader :legacy_artifacts_file, LegacyArtifactUploader, mount_on: :artifacts_file
mount_uploader :legacy_artifacts_metadata, LegacyArtifactUploader, mount_on: :artifacts_metadata mount_uploader :legacy_artifacts_metadata, LegacyArtifactUploader, mount_on: :artifacts_metadata
...@@ -775,7 +782,7 @@ module Ci ...@@ -775,7 +782,7 @@ module Ci
private private
def erase_old_artifacts! def erase_old_artifacts!
# TODO: To be removed once we get rid of # TODO: To be removed once we get rid of ci_enable_legacy_artifacts feature flag
remove_artifacts_file! remove_artifacts_file!
remove_artifacts_metadata! remove_artifacts_metadata!
save save
......
...@@ -13,7 +13,7 @@ module ArtifactMigratable ...@@ -13,7 +13,7 @@ module ArtifactMigratable
end end
def artifacts? def artifacts?
!artifacts_expired? && artifacts_file.exists? !artifacts_expired? && artifacts_file&.exists?
end end
def artifacts_metadata? def artifacts_metadata?
...@@ -43,4 +43,16 @@ module ArtifactMigratable ...@@ -43,4 +43,16 @@ module ArtifactMigratable
def artifacts_size def artifacts_size
read_attribute(:artifacts_size).to_i + job_artifacts.sum(:size).to_i read_attribute(:artifacts_size).to_i + job_artifacts.sum(:size).to_i
end end
def legacy_artifacts_file
return unless Feature.enabled?(:ci_enable_legacy_artifacts)
super
end
def legacy_artifacts_metadata
return unless Feature.enabled?(:ci_enable_legacy_artifacts)
super
end
end end
# frozen_string_literal: true # frozen_string_literal: true
##
# TODO: Remove this uploader when we remove :ci_enable_legacy_artifacts feature flag
# See https://gitlab.com/gitlab-org/gitlab-ce/issues/58595
class LegacyArtifactUploader < GitlabUploader class LegacyArtifactUploader < GitlabUploader
extend Workhorse::UploadPath extend Workhorse::UploadPath
include ObjectStorage::Concern include ObjectStorage::Concern
......
---
title: Drop legacy artifacts usage as there are no leftovers
merge_request: 24294
author:
type: performance
...@@ -47,7 +47,7 @@ module Gitlab ...@@ -47,7 +47,7 @@ module Gitlab
user: build.user.try(:hook_attrs), user: build.user.try(:hook_attrs),
runner: build.runner && runner_hook_attrs(build.runner), runner: build.runner && runner_hook_attrs(build.runner),
artifacts_file: { artifacts_file: {
filename: build.artifacts_file.filename, filename: build.artifacts_file&.filename,
size: build.artifacts_size size: build.artifacts_size
} }
} }
......
...@@ -117,6 +117,16 @@ describe Ci::Build do ...@@ -117,6 +117,16 @@ describe Ci::Build do
it 'returns the job' do it 'returns the job' do
is_expected.to include(job) is_expected.to include(job)
end end
context 'when ci_enable_legacy_artifacts feature flag is disabled' do
before do
stub_feature_flags(ci_enable_legacy_artifacts: false)
end
it 'does not return the job' do
is_expected.not_to include(job)
end
end
end end
context 'when job has a job artifact archive' do context 'when job has a job artifact archive' do
...@@ -471,6 +481,14 @@ describe Ci::Build do ...@@ -471,6 +481,14 @@ describe Ci::Build do
let(:build) { create(:ci_build, :legacy_artifacts) } let(:build) { create(:ci_build, :legacy_artifacts) }
it { is_expected.to be_truthy } it { is_expected.to be_truthy }
context 'when ci_enable_legacy_artifacts feature flag is disabled' do
before do
stub_feature_flags(ci_enable_legacy_artifacts: false)
end
it { is_expected.to be_falsy }
end
end end
end 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