From f079f01829f4f7b887d35375a5ce2a90afb98a15 Mon Sep 17 00:00:00 2001
From: Eduardo Bonet <ebonet@gitlab.com>
Date: Mon, 24 Jul 2023 07:25:42 +0000
Subject: [PATCH] Adds services to create Ml::* entities
Adds:
- Ml::FindOrCreateModelService
- Ml::FindOrCreateExperimentService
- Ml::FindOrCreateModelVersionService
Changelog: added
---
app/models/ml/experiment.rb | 4 ++
app/models/ml/model.rb | 5 ++
app/models/ml/model_version.rb | 6 +++
.../ml/find_or_create_experiment_service.rb | 19 +++++++
.../ml/find_or_create_model_service.rb | 22 ++++++++
.../find_or_create_model_version_service.rb | 22 ++++++++
spec/models/ml/experiment_spec.rb | 39 ++++++++++++++
spec/models/ml/model_spec.rb | 54 +++++++++++++++++--
spec/models/ml/model_version_spec.rb | 32 ++++++++++-
.../find_or_create_experiment_service_spec.rb | 33 ++++++++++++
.../ml/find_or_create_model_service_spec.rb | 45 ++++++++++++++++
...nd_or_create_model_version_service_spec.rb | 49 +++++++++++++++++
12 files changed, 324 insertions(+), 6 deletions(-)
create mode 100644 app/services/ml/find_or_create_experiment_service.rb
create mode 100644 app/services/ml/find_or_create_model_service.rb
create mode 100644 app/services/ml/find_or_create_model_version_service.rb
create mode 100644 spec/services/ml/find_or_create_experiment_service_spec.rb
create mode 100644 spec/services/ml/find_or_create_model_service_spec.rb
create mode 100644 spec/services/ml/find_or_create_model_version_service_spec.rb
diff --git a/app/models/ml/experiment.rb b/app/models/ml/experiment.rb
index 5c5f8d3b2db71..ad6c6b7b3bf00 100644
--- a/app/models/ml/experiment.rb
+++ b/app/models/ml/experiment.rb
@@ -59,6 +59,10 @@ def package_for_experiment?(package_name)
numeric?(iid)
end
+ def find_or_create(project, name, user)
+ create_with(user: user).find_or_create_by(project: project, name: name)
+ end
+
private
def numeric?(value)
diff --git a/app/models/ml/model.rb b/app/models/ml/model.rb
index 684b8e1983b70..f901eb69be4cb 100644
--- a/app/models/ml/model.rb
+++ b/app/models/ml/model.rb
@@ -21,5 +21,10 @@ def valid_default_experiment?
errors.add(:default_experiment) unless default_experiment.name == name
errors.add(:default_experiment) unless default_experiment.project_id == project_id
end
+
+ def self.find_or_create(project, name, experiment)
+ create_with(default_experiment: experiment)
+ .find_or_create_by(project: project, name: name)
+ end
end
end
diff --git a/app/models/ml/model_version.rb b/app/models/ml/model_version.rb
index 540fe6018a102..3450dc64b79f3 100644
--- a/app/models/ml/model_version.rb
+++ b/app/models/ml/model_version.rb
@@ -18,6 +18,12 @@ class ModelVersion < ApplicationRecord
delegate :name, to: :model
+ class << self
+ def find_or_create(model, version, package)
+ create_with(package: package).find_or_create_by(project: model.project, model: model, version: version)
+ end
+ end
+
private
def valid_model?
diff --git a/app/services/ml/find_or_create_experiment_service.rb b/app/services/ml/find_or_create_experiment_service.rb
new file mode 100644
index 0000000000000..1fe10c7f8562e
--- /dev/null
+++ b/app/services/ml/find_or_create_experiment_service.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+module Ml
+ class FindOrCreateExperimentService
+ def initialize(project, experiment_name, user = nil)
+ @project = project
+ @name = experiment_name
+ @user = user
+ end
+
+ def execute
+ Ml::Experiment.find_or_create(project, name, user)
+ end
+
+ private
+
+ attr_reader :project, :name, :user
+ end
+end
diff --git a/app/services/ml/find_or_create_model_service.rb b/app/services/ml/find_or_create_model_service.rb
new file mode 100644
index 0000000000000..66dec7a6234e7
--- /dev/null
+++ b/app/services/ml/find_or_create_model_service.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+module Ml
+ class FindOrCreateModelService
+ def initialize(project, model_name)
+ @project = project
+ @name = model_name
+ end
+
+ def execute
+ Ml::Model.find_or_create(
+ project,
+ name,
+ Ml::FindOrCreateExperimentService.new(project, name).execute
+ )
+ end
+
+ private
+
+ attr_reader :name, :project
+ end
+end
diff --git a/app/services/ml/find_or_create_model_version_service.rb b/app/services/ml/find_or_create_model_version_service.rb
new file mode 100644
index 0000000000000..558374ad57bd0
--- /dev/null
+++ b/app/services/ml/find_or_create_model_version_service.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+module Ml
+ class FindOrCreateModelVersionService
+ def initialize(project, params = {})
+ @project = project
+ @name = params[:model_name]
+ @version = params[:version]
+ @package = params[:package]
+ end
+
+ def execute
+ model = Ml::FindOrCreateModelService.new(project, name).execute
+
+ Ml::ModelVersion.find_or_create(model, version, package)
+ end
+
+ private
+
+ attr_reader :version, :name, :project, :package
+ end
+end
diff --git a/spec/models/ml/experiment_spec.rb b/spec/models/ml/experiment_spec.rb
index 1ee35d6da03a3..36bdb61183367 100644
--- a/spec/models/ml/experiment_spec.rb
+++ b/spec/models/ml/experiment_spec.rb
@@ -79,6 +79,45 @@
end
end
+ describe '.find_or_create' do
+ let(:name) { exp.name }
+ let(:project) { exp.project }
+
+ subject(:find_or_create) { described_class.find_or_create(project, name, exp.user) }
+
+ context 'when experiments exists' do
+ it 'fetches existing experiment', :aggregate_failures do
+ expect { find_or_create }.not_to change { Ml::Experiment.count }
+
+ expect(find_or_create).to eq(exp)
+ end
+ end
+
+ context 'when experiments does not exist' do
+ let(:name) { 'a new experiment' }
+
+ it 'creates the experiment', :aggregate_failures do
+ expect { find_or_create }.to change { Ml::Experiment.count }.by(1)
+
+ expect(find_or_create.name).to eq(name)
+ expect(find_or_create.user).to eq(exp.user)
+ expect(find_or_create.project).to eq(project)
+ end
+ end
+
+ context 'when experiment name exists but project is different' do
+ let(:project) { create(:project) }
+
+ it 'creates a model', :aggregate_failures do
+ expect { find_or_create }.to change { Ml::Experiment.count }.by(1)
+
+ expect(find_or_create.name).to eq(name)
+ expect(find_or_create.user).to eq(exp.user)
+ expect(find_or_create.project).to eq(project)
+ end
+ end
+ end
+
describe '#with_candidate_count' do
let_it_be(:exp3) do
create(:ml_experiments, project: exp.project).tap do |e|
diff --git a/spec/models/ml/model_spec.rb b/spec/models/ml/model_spec.rb
index 397ea23dd8554..cfe39239e8c14 100644
--- a/spec/models/ml/model_spec.rb
+++ b/spec/models/ml/model_spec.rb
@@ -3,6 +3,13 @@
require 'spec_helper'
RSpec.describe Ml::Model, feature_category: :mlops do
+ let_it_be(:base_project) { create(:project) }
+ let_it_be(:existing_model) { create(:ml_models, name: 'an_existing_model', project: base_project) }
+ let_it_be(:valid_name) { 'a_valid_name' }
+ let_it_be(:default_experiment) { create(:ml_experiments, name: valid_name, project: base_project) }
+
+ let(:project) { base_project }
+
describe 'associations' do
it { is_expected.to belong_to(:project) }
it { is_expected.to have_one(:default_experiment) }
@@ -12,11 +19,6 @@
describe '#valid?' do
using RSpec::Parameterized::TableSyntax
- let_it_be(:project) { create(:project) }
- let_it_be(:existing_model) { create(:ml_models, name: 'an_existing_model', project: project) }
- let_it_be(:valid_name) { 'a_valid_name' }
- let_it_be(:default_experiment) { create(:ml_experiments, name: valid_name, project: project) }
-
let(:name) { valid_name }
subject(:errors) do
@@ -59,4 +61,46 @@
end
end
end
+
+ describe '.find_or_create' do
+ subject(:find_or_create) { described_class.find_or_create(project, name, experiment) }
+
+ let(:name) { existing_model.name }
+ let(:project) { existing_model.project }
+ let(:experiment) { default_experiment }
+
+ context 'when model name does not exist in the project' do
+ let(:name) { 'new_model' }
+ let(:experiment) { build(:ml_experiments, name: name, project: project) }
+
+ it 'creates a model', :aggregate_failures do
+ expect { find_or_create }.to change { Ml::Model.count }.by(1)
+
+ expect(find_or_create.name).to eq(name)
+ expect(find_or_create.project).to eq(project)
+ expect(find_or_create.default_experiment).to eq(experiment)
+ end
+ end
+
+ context 'when model name exists but project is different' do
+ let(:project) { create(:project) }
+ let(:experiment) { build(:ml_experiments, name: name, project: project) }
+
+ it 'creates a model', :aggregate_failures do
+ expect { find_or_create }.to change { Ml::Model.count }.by(1)
+
+ expect(find_or_create.name).to eq(name)
+ expect(find_or_create.project).to eq(project)
+ expect(find_or_create.default_experiment).to eq(experiment)
+ end
+ end
+
+ context 'when model exists' do
+ it 'fetches existing model', :aggregate_failures do
+ expect { find_or_create }.not_to change { Ml::Model.count }
+
+ expect(find_or_create).to eq(existing_model)
+ end
+ end
+ end
end
diff --git a/spec/models/ml/model_version_spec.rb b/spec/models/ml/model_version_spec.rb
index ef53a1ac3a0ed..9af9a2a56c6c7 100644
--- a/spec/models/ml/model_version_spec.rb
+++ b/spec/models/ml/model_version_spec.rb
@@ -6,6 +6,7 @@
using RSpec::Parameterized::TableSyntax
let_it_be(:base_project) { create(:project) }
+ let_it_be(:model) { create(:ml_models, project: base_project) }
describe 'associations' do
it { is_expected.to belong_to(:project) }
@@ -15,7 +16,6 @@
describe 'validation' do
let_it_be(:valid_version) { 'valid_version' }
- let_it_be(:model) { create(:ml_models, project: base_project) }
let_it_be(:valid_package) do
build_stubbed(:ml_model_package, project: base_project, version: valid_version, name: model.name)
end
@@ -87,4 +87,34 @@
end
end
end
+
+ describe '.find_or_create' do
+ let_it_be(:existing_model_version) { create(:ml_model_versions, model: model, version: 'abc') }
+
+ let(:version) { existing_model_version.version }
+ let(:package) { nil }
+
+ subject(:find_or_create) { described_class.find_or_create(model, version, package) }
+
+ context 'if model version exists' do
+ it 'returns the model version', :aggregate_failures do
+ expect { find_or_create }.not_to change { Ml::ModelVersion.count }
+ is_expected.to eq(existing_model_version)
+ end
+ end
+
+ context 'if model version does not exist' do
+ let(:version) { 'new_version' }
+ let(:package) { create(:ml_model_package, project: model.project, name: model.name, version: version) }
+
+ it 'creates another model version', :aggregate_failures do
+ expect { find_or_create }.to change { Ml::ModelVersion.count }.by(1)
+ model_version = find_or_create
+
+ expect(model_version.version).to eq(version)
+ expect(model_version.model).to eq(model)
+ expect(model_version.package).to eq(package)
+ end
+ end
+ end
end
diff --git a/spec/services/ml/find_or_create_experiment_service_spec.rb b/spec/services/ml/find_or_create_experiment_service_spec.rb
new file mode 100644
index 0000000000000..a8c533d132035
--- /dev/null
+++ b/spec/services/ml/find_or_create_experiment_service_spec.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::Ml::FindOrCreateExperimentService, feature_category: :mlops do
+ let_it_be(:project) { create(:project) }
+ let_it_be(:user) { project.first_owner }
+ let_it_be(:existing_experiment) { create(:ml_experiments, project: project, user: user) }
+
+ let(:name) { 'new_experiment' }
+
+ subject(:new_experiment) { described_class.new(project, name, user).execute }
+
+ describe '#execute' do
+ it 'creates an experiment using Ml::Experiment.find_or_create', :aggregate_failures do
+ expect(Ml::Experiment).to receive(:find_or_create).and_call_original
+
+ expect(new_experiment.name).to eq('new_experiment')
+ expect(new_experiment.project).to eq(project)
+ expect(new_experiment.user).to eq(user)
+ end
+
+ context 'when experiment already exists' do
+ let(:name) { existing_experiment.name }
+
+ it 'fetches existing experiment', :aggregate_failures do
+ expect { new_experiment }.not_to change { Ml::Experiment.count }
+
+ expect(new_experiment).to eq(existing_experiment)
+ end
+ end
+ end
+end
diff --git a/spec/services/ml/find_or_create_model_service_spec.rb b/spec/services/ml/find_or_create_model_service_spec.rb
new file mode 100644
index 0000000000000..6ddae20f8d633
--- /dev/null
+++ b/spec/services/ml/find_or_create_model_service_spec.rb
@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::Ml::FindOrCreateModelService, feature_category: :mlops do
+ let_it_be(:existing_model) { create(:ml_models) }
+ let_it_be(:another_project) { create(:project) }
+
+ subject(:create_model) { described_class.new(project, name).execute }
+
+ describe '#execute' do
+ context 'when model name does not exist in the project' do
+ let(:name) { 'new_model' }
+ let(:project) { existing_model.project }
+
+ it 'creates a model', :aggregate_failures do
+ expect { create_model }.to change { Ml::Model.count }.by(1)
+
+ expect(create_model.name).to eq(name)
+ end
+ end
+
+ context 'when model name exists but project is different' do
+ let(:name) { existing_model.name }
+ let(:project) { another_project }
+
+ it 'creates a model', :aggregate_failures do
+ expect { create_model }.to change { Ml::Model.count }.by(1)
+
+ expect(create_model.name).to eq(name)
+ end
+ end
+
+ context 'when model with name exists' do
+ let(:name) { existing_model.name }
+ let(:project) { existing_model.project }
+
+ it 'fetches existing model', :aggregate_failures do
+ expect { create_model }.to change { Ml::Model.count }.by(0)
+
+ expect(create_model).to eq(existing_model)
+ end
+ end
+ end
+end
diff --git a/spec/services/ml/find_or_create_model_version_service_spec.rb b/spec/services/ml/find_or_create_model_version_service_spec.rb
new file mode 100644
index 0000000000000..5c29a3430d820
--- /dev/null
+++ b/spec/services/ml/find_or_create_model_version_service_spec.rb
@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::Ml::FindOrCreateModelVersionService, feature_category: :mlops do
+ let_it_be(:existing_version) { create(:ml_model_versions) }
+ let_it_be(:another_project) { create(:project) }
+
+ let(:package) { nil }
+
+ let(:params) do
+ {
+ model_name: name,
+ version: version,
+ package: package
+ }
+ end
+
+ subject(:model_version) { described_class.new(project, params).execute }
+
+ describe '#execute' do
+ context 'when model version exists' do
+ let(:name) { existing_version.name }
+ let(:version) { existing_version.version }
+ let(:project) { existing_version.project }
+
+ it 'returns existing model version', :aggregate_failures do
+ expect { model_version }.to change { Ml::ModelVersion.count }.by(0)
+ expect(model_version).to eq(existing_version)
+ end
+ end
+
+ context 'when model version does not exist' do
+ let(:project) { existing_version.project }
+ let(:name) { 'a_new_model' }
+ let(:version) { 'a_new_version' }
+
+ let(:package) { create(:ml_model_package, project: project, name: name, version: version) }
+
+ it 'creates a new model version', :aggregate_failures do
+ expect { model_version }.to change { Ml::ModelVersion.count }
+
+ expect(model_version.name).to eq(name)
+ expect(model_version.version).to eq(version)
+ expect(model_version.package).to eq(package)
+ end
+ end
+ end
+end
--
GitLab