Pipeline Artifacts domain specific logic is spilled to the other domain
Problem
The following discussion from !39721 (merged) should be addressed:
-
@dosuken123 started a discussion: (+3 comments) Probably we should update
MergeRequest#has_coverage_reports?
toactual_head_pipeline&.pipeline_artifacts.exists?(file_type: :code_coverage)
thanactual_head_pipeline&.has_reports?(Ci::JobArtifact.coverage_reports)
, as the latter one doesn't guarantee the pipeline artifact existence. SincePipelines::CreateArtifactWorker
worker runs onpipeline_background
sidekiq queue namespace, which is the lowest prioriy, there would be a latency frompipeline.complete? == true
to the actual artifact creation.
Solution
Looking at
actual_head_pipeline&.pipeline_artifacts&.has_code_coverage?
, It seems we're spilling the domain specific logic in MergeRequest. Ideally,pipeline_artifacts
should be abstracted under theCi::Pipeline
model.
Can we improve the naming of the interfaces to make it more explicit and comprehensive, for example:
-
Ci::Pipeline#has_coverage_reports?
=>pipeline_artifacts&.has_code_coverage?
-
Ci::Pipeline#can_generate_coverage_reports?
=>has_reports?(Ci::JobArtifact.coverage_reports)
-
Ci::PipelineArtifact#has_code_coverage?
=> as-is