From a9ab01ab6eac34e09b0d7cf01e1e352680045ec2 Mon Sep 17 00:00:00 2001
From: Eduardo Bonet <ebonet@gitlab.com>
Date: Tue, 5 Dec 2023 14:22:49 +0000
Subject: [PATCH] Adds candidate info to model version view model
Reuses CandidateDetailPresenter to include a model versions candidate
info and display in the model page and model detail page.
---
.../components/model_version_detail.vue | 21 +++++++-
.../ml/model_registry/translations.js | 2 +
.../projects/ml/show_ml_model_component.rb | 8 +--
.../ml/show_ml_model_version_component.rb | 12 +++--
.../find_or_create_model_version_service.rb | 10 ++--
.../projects/ml/model_versions/show.html.haml | 2 +-
app/views/projects/ml/models/show.html.haml | 2 +-
locale/gitlab.pot | 3 ++
.../ml/show_ml_model_component_spec.rb | 26 ++++++++--
.../show_ml_model_version_component_spec.rb | 31 +++++++++--
spec/factories/ml/model_versions.rb | 4 ++
.../components/model_version_detail_spec.js | 52 ++++++++++++-------
spec/frontend/ml/model_registry/mock_data.js | 10 +++-
spec/models/ml/candidate_spec.rb | 4 +-
.../ml/create_candidate_service_spec.rb | 2 +-
15 files changed, 144 insertions(+), 45 deletions(-)
diff --git a/app/assets/javascripts/ml/model_registry/components/model_version_detail.vue b/app/assets/javascripts/ml/model_registry/components/model_version_detail.vue
index 19d91df43b250..8d3e8cf2023cc 100644
--- a/app/assets/javascripts/ml/model_registry/components/model_version_detail.vue
+++ b/app/assets/javascripts/ml/model_registry/components/model_version_detail.vue
@@ -1,12 +1,15 @@
<script>
import { convertToGraphQLId } from '~/graphql_shared/utils';
import { TYPENAME_PACKAGES_PACKAGE } from '~/graphql_shared/constants';
+import * as i18n from '../translations';
+import CandidateDetail from './candidate_detail.vue';
export default {
name: 'ModelVersionDetail',
components: {
PackageFiles: () =>
import('~/packages_and_registries/package_registry/components/details/package_files.vue'),
+ CandidateDetail,
},
props: {
modelVersion: {
@@ -25,14 +28,21 @@ export default {
return 'ml_model';
},
},
+ i18n,
};
</script>
<template>
<div>
- <p>
+ <h3 class="gl-font-lg gl-mt-5">{{ $options.i18n.DESCRIPTION_LABEL }}</h3>
+
+ <div v-if="modelVersion.description">
{{ modelVersion.description }}
- </p>
+ </div>
+ <div v-else class="gl-text-secondary">
+ {{ $options.i18n.NO_DESCRIPTION_PROVIDED_LABEL }}
+ </div>
+
<template v-if="modelVersion.packageId">
<package-files
:package-id="packageId"
@@ -40,5 +50,12 @@ export default {
:package-type="packageType"
/>
</template>
+
+ <div class="gl-mt-5">
+ <span class="gl-font-weight-bold">{{ $options.i18n.MLFLOW_ID_LABEL }}:</span>
+ {{ modelVersion.candidate.info.eid }}
+ </div>
+
+ <candidate-detail :candidate="modelVersion.candidate" :show-info-section="false" />
</div>
</template>
diff --git a/app/assets/javascripts/ml/model_registry/translations.js b/app/assets/javascripts/ml/model_registry/translations.js
index bbafde0b94338..f4ff0f4aa973d 100644
--- a/app/assets/javascripts/ml/model_registry/translations.js
+++ b/app/assets/javascripts/ml/model_registry/translations.js
@@ -15,6 +15,8 @@ export const NO_MODELS_LABEL = s__('MlModelRegistry|No models registered in this
export const modelsCountLabel = (modelCount) =>
n__('MlModelRegistry|%d model', 'MlModelRegistry|%d models', modelCount);
+export const DESCRIPTION_LABEL = __('Description');
+export const NO_DESCRIPTION_PROVIDED_LABEL = s__('MlModelRegistry|No description provided');
export const INFO_LABEL = s__('MlModelRegistry|Info');
export const ID_LABEL = s__('MlModelRegistry|ID');
export const MLFLOW_ID_LABEL = s__('MlModelRegistry|MLflow run ID');
diff --git a/app/components/projects/ml/show_ml_model_component.rb b/app/components/projects/ml/show_ml_model_component.rb
index 38a81a5837d02..03300f01f6445 100644
--- a/app/components/projects/ml/show_ml_model_component.rb
+++ b/app/components/projects/ml/show_ml_model_component.rb
@@ -3,10 +3,11 @@
module Projects
module Ml
class ShowMlModelComponent < ViewComponent::Base
- attr_reader :model
+ attr_reader :model, :current_user
- def initialize(model:)
+ def initialize(model:, current_user:)
@model = model.present
+ @current_user = current_user
end
private
@@ -35,7 +36,8 @@ def latest_version_view_model
version: model_version.version,
description: model_version.description,
project_path: project_path(model_version.project),
- package_id: model_version.package_id
+ package_id: model_version.package_id,
+ **::Ml::CandidateDetailsPresenter.new(model_version.candidate, current_user).present
}
end
end
diff --git a/app/components/projects/ml/show_ml_model_version_component.rb b/app/components/projects/ml/show_ml_model_version_component.rb
index 0e39a1cbcc62a..a4c641f6d66a6 100644
--- a/app/components/projects/ml/show_ml_model_version_component.rb
+++ b/app/components/projects/ml/show_ml_model_version_component.rb
@@ -3,11 +3,12 @@
module Projects
module Ml
class ShowMlModelVersionComponent < ViewComponent::Base
- attr_reader :model_version, :model
+ attr_reader :model_version, :model, :current_user
- def initialize(model_version:)
+ def initialize(model_version:, current_user:)
@model_version = model_version.present
@model = model_version.model.present
+ @current_user = current_user
end
private
@@ -24,12 +25,17 @@ def view_model
model: {
name: model.name,
path: model.path
- }
+ },
+ **candidate_data
}
}
Gitlab::Json.generate(vm.deep_transform_keys { |k| k.to_s.camelize(:lower) })
end
+
+ def candidate_data
+ ::Ml::CandidateDetailsPresenter.new(model_version.candidate, current_user).present
+ end
end
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
index a5e9bf997cc9c..1c6f5bb96dd60 100644
--- a/app/services/ml/find_or_create_model_version_service.rb
+++ b/app/services/ml/find_or_create_model_version_service.rb
@@ -15,10 +15,12 @@ def execute
model_version = Ml::ModelVersion.find_or_create!(model, @version, @package, @description)
- model_version.candidate = ::Ml::CreateCandidateService.new(
- model.default_experiment,
- { model_version: model_version }
- ).execute
+ unless model_version.candidate
+ model_version.candidate = ::Ml::CreateCandidateService.new(
+ model.default_experiment,
+ { model_version: model_version }
+ ).execute
+ end
model_version
end
diff --git a/app/views/projects/ml/model_versions/show.html.haml b/app/views/projects/ml/model_versions/show.html.haml
index 0b3d5462a8968..1b4bdd29842d8 100644
--- a/app/views/projects/ml/model_versions/show.html.haml
+++ b/app/views/projects/ml/model_versions/show.html.haml
@@ -3,4 +3,4 @@
- breadcrumb_title @model_version.version
- page_title "#{@model_version.name} / #{@model_version.version}"
-= render(Projects::Ml::ShowMlModelVersionComponent.new(model_version: @model_version))
+= render(Projects::Ml::ShowMlModelVersionComponent.new(model_version: @model_version, current_user: current_user))
diff --git a/app/views/projects/ml/models/show.html.haml b/app/views/projects/ml/models/show.html.haml
index be611e55304dc..e0067143450c1 100644
--- a/app/views/projects/ml/models/show.html.haml
+++ b/app/views/projects/ml/models/show.html.haml
@@ -2,4 +2,4 @@
- breadcrumb_title @model.name
- page_title @model.name
-= render(Projects::Ml::ShowMlModelComponent.new(model: @model))
+= render(Projects::Ml::ShowMlModelComponent.new(model: @model, current_user: current_user))
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 29ad8b192e2f1..2a5a78b5990e7 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -30847,6 +30847,9 @@ msgstr ""
msgid "MlModelRegistry|Model registry"
msgstr ""
+msgid "MlModelRegistry|No description provided"
+msgstr ""
+
msgid "MlModelRegistry|No logged metadata"
msgstr ""
diff --git a/spec/components/projects/ml/show_ml_model_component_spec.rb b/spec/components/projects/ml/show_ml_model_component_spec.rb
index 02fad55e0be4f..ed9894216794c 100644
--- a/spec/components/projects/ml/show_ml_model_component_spec.rb
+++ b/spec/components/projects/ml/show_ml_model_component_spec.rb
@@ -3,11 +3,14 @@
require "spec_helper"
RSpec.describe Projects::Ml::ShowMlModelComponent, type: :component, feature_category: :mlops do
- let_it_be(:project) { build_stubbed(:project) }
- let_it_be(:model1) { build_stubbed(:ml_models, :with_latest_version_and_package, project: project) }
+ let_it_be(:project) { create(:project) } # rubocop:disable RSpec/FactoryBot/AvoidCreate -- build_stubbed breaks because it doesn't create iids properly.
+ let_it_be(:model1) { create(:ml_models, :with_latest_version_and_package, project: project) } # rubocop:disable RSpec/FactoryBot/AvoidCreate -- build_stubbed breaks because it doesn't create iids properly.
+
+ let_it_be(:experiment) { model1.default_experiment }
+ let_it_be(:candidate) { model1.latest_version.candidate }
subject(:component) do
- described_class.new(model: model1)
+ described_class.new(model: model1, current_user: model1.user)
end
describe 'rendered' do
@@ -28,7 +31,22 @@
'version' => model1.latest_version.version,
'description' => model1.latest_version.description,
'projectPath' => "/#{project.full_path}",
- 'packageId' => model1.latest_version.package_id
+ 'packageId' => model1.latest_version.package_id,
+ 'candidate' => {
+ 'info' => {
+ 'iid' => candidate.iid,
+ 'eid' => candidate.eid,
+ 'pathToArtifact' => nil,
+ 'experimentName' => candidate.experiment.name,
+ 'pathToExperiment' => "/#{project.full_path}/-/ml/experiments/#{experiment.iid}",
+ 'status' => 'running',
+ 'path' => "/#{project.full_path}/-/ml/candidates/#{candidate.iid}",
+ 'ciJob' => nil
+ },
+ 'metrics' => [],
+ 'params' => [],
+ 'metadata' => []
+ }
},
'versionCount' => 1
}
diff --git a/spec/components/projects/ml/show_ml_model_version_component_spec.rb b/spec/components/projects/ml/show_ml_model_version_component_spec.rb
index a7dad5e4b2baf..89f0c8633c27b 100644
--- a/spec/components/projects/ml/show_ml_model_version_component_spec.rb
+++ b/spec/components/projects/ml/show_ml_model_version_component_spec.rb
@@ -3,12 +3,20 @@
require "spec_helper"
RSpec.describe Projects::Ml::ShowMlModelVersionComponent, type: :component, feature_category: :mlops do
- let_it_be(:project) { build_stubbed(:project) }
- let_it_be(:model) { build_stubbed(:ml_models, project: project) }
- let_it_be(:version) { build_stubbed(:ml_model_versions, :with_package, model: model, description: 'abc') }
+ let_it_be(:project) { create(:project) } # rubocop:disable RSpec/FactoryBot/AvoidCreate -- build_stubbed breaks because it doesn't create iids properly.
+ let_it_be(:user) { project.owner }
+ let_it_be(:model) { create(:ml_models, project: project) } # rubocop:disable RSpec/FactoryBot/AvoidCreate -- build_stubbed breaks because it doesn't create iids properly.
+ let_it_be(:experiment) { model.default_experiment }
+ let_it_be(:candidate) do
+ create(:ml_candidates, :with_artifact, experiment: experiment, user: user, project: project) # rubocop:disable RSpec/FactoryBot/AvoidCreate -- build_stubbed breaks because it doesn't create iids properly.
+ end
+
+ let_it_be(:version) do
+ build_stubbed(:ml_model_versions, :with_package, model: model, candidate: candidate, description: 'abc')
+ end
subject(:component) do
- described_class.new(model_version: version)
+ described_class.new(model_version: version, current_user: user)
end
describe 'rendered' do
@@ -30,6 +38,21 @@
'model' => {
'name' => model.name,
'path' => "/#{project.full_path}/-/ml/models/#{model.id}"
+ },
+ 'candidate' => {
+ 'info' => {
+ 'iid' => candidate.iid,
+ 'eid' => candidate.eid,
+ 'pathToArtifact' => "/#{project.full_path}/-/packages/#{candidate.artifact.id}",
+ 'experimentName' => candidate.experiment.name,
+ 'pathToExperiment' => "/#{project.full_path}/-/ml/experiments/#{experiment.iid}",
+ 'status' => 'running',
+ 'path' => "/#{project.full_path}/-/ml/candidates/#{candidate.iid}",
+ 'ciJob' => nil
+ },
+ 'metrics' => [],
+ 'params' => [],
+ 'metadata' => []
}
}
})
diff --git a/spec/factories/ml/model_versions.rb b/spec/factories/ml/model_versions.rb
index a097640b1345a..fd7ed857ee272 100644
--- a/spec/factories/ml/model_versions.rb
+++ b/spec/factories/ml/model_versions.rb
@@ -8,6 +8,10 @@
project { model.project }
description { 'Some description' }
+ candidate do
+ association :ml_candidates, experiment: model.default_experiment, project: project, model_version: instance
+ end
+
trait :with_package do
package do
association :ml_model_package, name: model.name, version: version, project: project
diff --git a/spec/frontend/ml/model_registry/components/model_version_detail_spec.js b/spec/frontend/ml/model_registry/components/model_version_detail_spec.js
index aeb9d13ad9783..d1874346ad740 100644
--- a/spec/frontend/ml/model_registry/components/model_version_detail_spec.js
+++ b/spec/frontend/ml/model_registry/components/model_version_detail_spec.js
@@ -3,6 +3,7 @@ import Vue from 'vue';
import VueApollo from 'vue-apollo';
import ModelVersionDetail from '~/ml/model_registry/components/model_version_detail.vue';
import PackageFiles from '~/packages_and_registries/package_registry/components/details/package_files.vue';
+import CandidateDetail from '~/ml/model_registry/components/candidate_detail.vue';
import createMockApollo from 'helpers/mock_apollo_helper';
import { makeModelVersion, MODEL_VERSION } from '../mock_data';
@@ -15,36 +16,51 @@ const createWrapper = (modelVersion = MODEL_VERSION) => {
};
const findPackageFiles = () => wrapper.findComponent(PackageFiles);
+const findCandidateDetail = () => wrapper.findComponent(CandidateDetail);
describe('ml/model_registry/components/model_version_detail.vue', () => {
- describe('description', () => {
+ describe('base behaviour', () => {
beforeEach(() => createWrapper());
it('shows the description', () => {
expect(wrapper.text()).toContain(MODEL_VERSION.description);
});
- });
- describe('package files', () => {
- describe('if package exists', () => {
- beforeEach(() => createWrapper());
-
- it('renders files', () => {
- expect(findPackageFiles().props()).toEqual({
- packageId: 'gid://gitlab/Packages::Package/12',
- projectPath: MODEL_VERSION.projectPath,
- packageType: 'ml_model',
- canDelete: false,
- });
- });
+ it('shows the candidate', () => {
+ expect(findCandidateDetail().props('candidate')).toBe(MODEL_VERSION.candidate);
});
- describe('if package does not exist', () => {
- beforeEach(() => createWrapper(makeModelVersion({ packageId: 0 })));
+ it('shows the mlflow label string', () => {
+ expect(wrapper.text()).toContain('MLflow run ID');
+ });
- it('does not render files', () => {
- expect(findPackageFiles().exists()).toBe(false);
+ it('shows the mlflow id', () => {
+ expect(wrapper.text()).toContain(MODEL_VERSION.candidate.info.eid);
+ });
+
+ it('renders files', () => {
+ expect(findPackageFiles().props()).toEqual({
+ packageId: 'gid://gitlab/Packages::Package/12',
+ projectPath: MODEL_VERSION.projectPath,
+ packageType: 'ml_model',
+ canDelete: false,
});
});
});
+
+ describe('if package does not exist', () => {
+ beforeEach(() => createWrapper(makeModelVersion({ packageId: 0 })));
+
+ it('does not render files', () => {
+ expect(findPackageFiles().exists()).toBe(false);
+ });
+ });
+
+ describe('if model version does not have description', () => {
+ beforeEach(() => createWrapper(makeModelVersion({ description: null })));
+
+ it('renders no description provided label', () => {
+ expect(wrapper.text()).toContain('No description provided');
+ });
+ });
});
diff --git a/spec/frontend/ml/model_registry/mock_data.js b/spec/frontend/ml/model_registry/mock_data.js
index 07cf59388ea59..78e22eda7b9b0 100644
--- a/spec/frontend/ml/model_registry/mock_data.js
+++ b/spec/frontend/ml/model_registry/mock_data.js
@@ -56,12 +56,18 @@ export const makeModel = ({ latestVersion } = { latestVersion: LATEST_VERSION })
export const MODEL = makeModel();
-export const makeModelVersion = ({ version = '1.2.3', model = MODEL, packageId = 12 } = {}) => ({
+export const makeModelVersion = ({
+ version = '1.2.3',
+ model = MODEL,
+ packageId = 12,
+ description = 'Model version description',
+} = {}) => ({
version,
model,
packageId,
- description: 'Model version description',
+ description,
projectPath: 'path/to/project',
+ candidate: newCandidate(),
});
export const MODEL_VERSION = makeModelVersion();
diff --git a/spec/models/ml/candidate_spec.rb b/spec/models/ml/candidate_spec.rb
index 503d3514a72e4..678224a3c8eff 100644
--- a/spec/models/ml/candidate_spec.rb
+++ b/spec/models/ml/candidate_spec.rb
@@ -38,8 +38,8 @@
describe 'validation' do
let_it_be(:model) { create(:ml_models, project: candidate.project) }
- let_it_be(:model_version1) { create(:ml_model_versions, model: model) }
- let_it_be(:model_version2) { create(:ml_model_versions, model: model) }
+ let_it_be(:model_version1) { create(:ml_model_versions, model: model, candidate: nil) }
+ let_it_be(:model_version2) { create(:ml_model_versions, model: model, candidate: nil) }
let_it_be(:validation_candidate) do
create(:ml_candidates, model_version: model_version1, project: candidate.project)
end
diff --git a/spec/services/ml/create_candidate_service_spec.rb b/spec/services/ml/create_candidate_service_spec.rb
index fb3456b0bccc4..b1a053711d72d 100644
--- a/spec/services/ml/create_candidate_service_spec.rb
+++ b/spec/services/ml/create_candidate_service_spec.rb
@@ -4,7 +4,7 @@
RSpec.describe ::Ml::CreateCandidateService, feature_category: :mlops do
describe '#execute' do
- let_it_be(:model_version) { create(:ml_model_versions) }
+ let_it_be(:model_version) { create(:ml_model_versions, candidate: nil) }
let_it_be(:experiment) { create(:ml_experiments, project: model_version.project) }
let(:params) { {} }
--
GitLab