diff --git a/app/models/concerns/enums/internal_id.rb b/app/models/concerns/enums/internal_id.rb index 71c86bab1361d991375ab577309d023a75f2d2b4..a8227363a22a9df61ae8cc086e8d4630fee4d332 100644 --- a/app/models/concerns/enums/internal_id.rb +++ b/app/models/concerns/enums/internal_id.rb @@ -16,7 +16,8 @@ def self.usage_resources alert_management_alerts: 8, sprints: 9, # iterations design_management_designs: 10, - incident_management_oncall_schedules: 11 + incident_management_oncall_schedules: 11, + ml_experiments: 12 } end end diff --git a/app/models/ml/experiment.rb b/app/models/ml/experiment.rb index 7ef9c70ba7ee69fa715642d69bd418cf9a429885..e4e9baac4c8cd31cb448234f3b7b9182b65ac7fe 100644 --- a/app/models/ml/experiment.rb +++ b/app/models/ml/experiment.rb @@ -2,11 +2,33 @@ module Ml class Experiment < ApplicationRecord - validates :name, :iid, :project, presence: true - validates :iid, :name, uniqueness: { scope: :project, message: "should be unique in the project" } + include AtomicInternalId + + validates :name, :project, presence: true + validates :name, uniqueness: { scope: :project, message: "should be unique in the project" } belongs_to :project belongs_to :user has_many :candidates, class_name: 'Ml::Candidate' + + has_internal_id :iid, scope: :project + + def artifact_location + 'not_implemented' + end + + class << self + def by_project_id_and_iid(project_id, iid) + find_by(project_id: project_id, iid: iid) + end + + def by_project_id_and_name(project_id, name) + find_by(project_id: project_id, name: name) + end + + def has_record?(project_id, name) + where(project_id: project_id, name: name).exists? + end + end end end diff --git a/config/feature_flags/development/ml_experiment_tracking.yml b/config/feature_flags/development/ml_experiment_tracking.yml new file mode 100644 index 0000000000000000000000000000000000000000..2749cbc3fc1e4ffdb04c3b1e8aaa7994647f95b3 --- /dev/null +++ b/config/feature_flags/development/ml_experiment_tracking.yml @@ -0,0 +1,8 @@ +--- +name: ml_experiment_tracking +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/95689 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/371669 +milestone: '15.4' +type: development +group: group::incubation +default_enabled: false diff --git a/db/migrate/20220818132108_add_deleted_on_to_ml_experiments.rb b/db/migrate/20220818132108_add_deleted_on_to_ml_experiments.rb new file mode 100644 index 0000000000000000000000000000000000000000..e6ba9f78553b3abb5c87ae4ceaa47724657570a2 --- /dev/null +++ b/db/migrate/20220818132108_add_deleted_on_to_ml_experiments.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddDeletedOnToMlExperiments < Gitlab::Database::Migration[2.0] + def change + add_column :ml_experiments, :deleted_on, :datetime_with_timezone, index: true + end +end diff --git a/db/schema_migrations/20220818132108 b/db/schema_migrations/20220818132108 new file mode 100644 index 0000000000000000000000000000000000000000..77683e61f2e4bc0fabdab03ac8643e3c2a0e61d5 --- /dev/null +++ b/db/schema_migrations/20220818132108 @@ -0,0 +1 @@ +7abea29f31054d1e0337d3fa434f55cc1c354701da89e257c764b85cd2cc2768 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index ae1b0ca99902c042655d3b9560f921073cca1420..e9ec938289d4c16bc8cebe0bb12fd4d16164c977 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -17718,6 +17718,7 @@ CREATE TABLE ml_experiments ( project_id bigint NOT NULL, user_id bigint, name text NOT NULL, + deleted_on timestamp with time zone, CONSTRAINT check_ee07a0be2c CHECK ((char_length(name) <= 255)) ); diff --git a/lib/api/api.rb b/lib/api/api.rb index 5a8772d6c568b9240e9034c6b9129066739f95b8..443bf1d649aace1372c6c8e3761e9e29eabd7561 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -318,6 +318,7 @@ class API < ::API::Base mount ::API::Users mount ::API::Version mount ::API::Wikis + mount ::API::Ml::Mlflow end mount ::API::Internal::Base diff --git a/lib/api/entities/ml/mlflow/get_experiment.rb b/lib/api/entities/ml/mlflow/get_experiment.rb new file mode 100644 index 0000000000000000000000000000000000000000..cd42c45c06b71e5eb31c1bf445cd105089cd1488 --- /dev/null +++ b/lib/api/entities/ml/mlflow/get_experiment.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module API + module Entities + module Ml + module Mlflow + class GetExperiment < Grape::Entity + expose :experiment do + expose :experiment_id + expose :name + expose :lifecycle_stage + expose :artifact_location + end + + private + + def lifecycle_stage + object.deleted_on? ? 'deleted' : 'active' + end + + def experiment_id + object.iid.to_s + end + end + end + end + end +end diff --git a/lib/api/entities/ml/mlflow/new_experiment.rb b/lib/api/entities/ml/mlflow/new_experiment.rb new file mode 100644 index 0000000000000000000000000000000000000000..0979183985067f7db4a4728a8d1dccf19bc610c4 --- /dev/null +++ b/lib/api/entities/ml/mlflow/new_experiment.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module API + module Entities + module Ml + module Mlflow + class NewExperiment < Grape::Entity + expose :experiment_id + + private + + def experiment_id + object.iid.to_s + end + end + end + end + end +end diff --git a/lib/api/ml/mlflow.rb b/lib/api/ml/mlflow.rb new file mode 100644 index 0000000000000000000000000000000000000000..dbdfa03411453d0b8806ba1f50b1ee179ead81cb --- /dev/null +++ b/lib/api/ml/mlflow.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +require 'mime/types' + +module API + # MLFlow integration API, replicating the Rest API https://www.mlflow.org/docs/latest/rest-api.html#rest-api + module Ml + class Mlflow < ::API::Base + # The first part of the url is the namespace, the second part of the URL is what the MLFlow client calls + MLFLOW_API_PREFIX = ':id/ml/mflow/api/2.0/mlflow/' + + before do + authenticate! + not_found! unless Feature.enabled?(:ml_experiment_tracking, user_project) + end + + feature_category :mlops + + content_type :json, 'application/json' + default_format :json + + helpers do + def resource_not_found! + render_structured_api_error!({ error_code: 'RESOURCE_DOES_NOT_EXIST' }, 404) + end + + def resource_already_exists! + render_structured_api_error!({ error_code: 'RESOURCE_ALREADY_EXISTS' }, 400) + end + end + + params do + requires :id, type: String, desc: 'The ID of a project' + end + resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do + desc 'API to interface with MLFlow Client, REST API version 1.28.0' do + detail 'This feature is gated by :ml_experiment_tracking.' + end + namespace MLFLOW_API_PREFIX do + resource :experiments do + desc 'Fetch experiment by experiment_id' do + success Entities::Ml::Mlflow::GetExperiment + detail 'https://www.mlflow.org/docs/1.28.0/rest-api.html#get-experiment' + end + params do + optional :experiment_id, type: String, default: '', desc: 'Experiment ID, in reference to the project' + end + get 'get', urgency: :low do + experiment = ::Ml::Experiment.by_project_id_and_iid(user_project.id, params[:experiment_id]) + + resource_not_found! unless experiment + + present experiment, with: Entities::Ml::Mlflow::GetExperiment + end + + desc 'Fetch experiment by experiment_name' do + success Entities::Ml::Mlflow::GetExperiment + detail 'https://www.mlflow.org/docs/1.28.0/rest-api.html#get-experiment-by-name' + end + params do + optional :experiment_name, type: String, default: '', desc: 'Experiment name' + end + get 'get-by-name', urgency: :low do + experiment = ::Ml::Experiment.by_project_id_and_name(user_project, params[:experiment_name]) + + resource_not_found! unless experiment + + present experiment, with: Entities::Ml::Mlflow::GetExperiment + end + + desc 'Create experiment' do + success Entities::Ml::Mlflow::NewExperiment + detail 'https://www.mlflow.org/docs/1.28.0/rest-api.html#create-experiment' + end + params do + requires :name, type: String, desc: 'Experiment name' + optional :artifact_location, type: String, desc: 'This will be ignored' + optional :tags, type: Array, desc: 'This will be ignored' + end + post 'create', urgency: :low do + resource_already_exists! if ::Ml::Experiment.has_record?(user_project.id, params[:name]) + + experiment = ::Ml::Experiment.create!(name: params[:name], + user: current_user, + project: user_project) + + present experiment, with: Entities::Ml::Mlflow::NewExperiment + end + end + end + end + end + end +end diff --git a/spec/factories/ml/experiments.rb b/spec/factories/ml/experiments.rb new file mode 100644 index 0000000000000000000000000000000000000000..043ca712e60886997d9e1dd32387a1facc4bca0b --- /dev/null +++ b/spec/factories/ml/experiments.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true +FactoryBot.define do + factory :ml_experiments, class: '::Ml::Experiment' do + sequence(:name) { |n| "experiment#{n}" } + association :project + association :user + end +end diff --git a/spec/fixtures/api/schemas/ml/get_experiment.json b/spec/fixtures/api/schemas/ml/get_experiment.json new file mode 100644 index 0000000000000000000000000000000000000000..cf8da7f999f4df8ac4f0f214905d5ae3c3a64077 --- /dev/null +++ b/spec/fixtures/api/schemas/ml/get_experiment.json @@ -0,0 +1,23 @@ +{ + "type": "object", + "required": [ + "experiment" + ], + "properties": { + "experiment": { + "type": "object", + "required" : [ + "experiment_id", + "name", + "artifact_location", + "lifecycle_stage" + ], + "properties" : { + "experiment_id": { "type": "string" }, + "name": { "type": "string" }, + "artifact_location": { "type": "string" }, + "lifecycle_stage": { "type": { "enum" : ["active", "deleted"] } } + } + } + } +} diff --git a/spec/models/ml/experiment_spec.rb b/spec/models/ml/experiment_spec.rb index dca5280a8fe7948604618b191dc24f2d1571945f..e300f82d2904ff03a600dca504a83ac60bbad28d 100644 --- a/spec/models/ml/experiment_spec.rb +++ b/spec/models/ml/experiment_spec.rb @@ -8,4 +8,55 @@ it { is_expected.to belong_to(:user) } it { is_expected.to have_many(:candidates) } end + + describe '#by_project_id_and_iid?' do + let(:exp) { create(:ml_experiments) } + let(:iid) { exp.iid } + + subject { described_class.by_project_id_and_iid(exp.project_id, iid) } + + context 'if exists' do + it { is_expected.to eq(exp) } + end + + context 'if does not exist' do + let(:iid) { non_existing_record_id } + + it { is_expected.to be(nil) } + end + end + + describe '#by_project_id_and_name?' do + let(:exp) { create(:ml_experiments) } + let(:exp_name) { exp.name } + + subject { described_class.by_project_id_and_name(exp.project_id, exp_name) } + + context 'if exists' do + it { is_expected.to eq(exp) } + end + + context 'if does not exist' do + let(:exp_name) { 'hello' } + + it { is_expected.to be_nil } + end + end + + describe '#has_record?' do + let(:exp) { create(:ml_experiments) } + let(:exp_name) { exp.name } + + subject { described_class.has_record?(exp.project_id, exp_name) } + + context 'if exists' do + it { is_expected.to be_truthy } + end + + context 'if does not exist' do + let(:exp_name) { 'hello' } + + it { is_expected.to be_falsey } + end + end end diff --git a/spec/requests/api/ml/mlflow_spec.rb b/spec/requests/api/ml/mlflow_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..534947fcac7c819f89563ffb08656a317dbe6862 --- /dev/null +++ b/spec/requests/api/ml/mlflow_spec.rb @@ -0,0 +1,188 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'mime/types' + +RSpec.describe API::Ml::Mlflow do + include SessionHelpers + include ApiHelpers + include HttpBasicAuthHelpers + + let_it_be(:project) { create(:project, :private) } + let_it_be(:developer) { create(:user).tap { |u| project.add_developer(u) } } + let_it_be(:experiment) do + create(:ml_experiments, user: project.creator, project: project) + end + + let(:current_user) { developer } + let(:ff_value) { true } + let(:scopes) { %w[api] } + let(:headers) do + { 'Authorization' => "Bearer #{create(:personal_access_token, scopes: scopes, user: current_user).token}" } + end + + let(:params) { {} } + let(:request) { get api(route), params: params, headers: headers } + + before do + stub_feature_flags(ml_experiment_tracking: ff_value) + + request + end + + shared_examples 'Not Found' do |message| + it "is Not Found" do + expect(response).to have_gitlab_http_status(:not_found) + + expect(json_response['message']).to eq(message) if message.present? + end + end + + shared_examples 'Not Found - Resource Does Not Exist' do + it "is Resource Does Not Exist" do + expect(response).to have_gitlab_http_status(:not_found) + + expect(json_response).to include({ "error_code" => 'RESOURCE_DOES_NOT_EXIST' }) + end + end + + shared_examples 'Bad Request' do |error_code = nil| + it "is Bad Request" do + expect(response).to have_gitlab_http_status(:bad_request) + + expect(json_response).to include({ 'error_code' => error_code }) if error_code.present? + end + end + + shared_examples 'shared error cases' do + context 'when not authenticated' do + let(:headers) { {} } + + it "is Unauthorized" do + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'when user does not have access' do + let(:current_user) { create(:user) } + + it_behaves_like 'Not Found' + end + + context 'when ff is disabled' do + let(:ff_value) { false } + + it_behaves_like 'Not Found' + end + end + + describe 'GET /projects/:id/ml/mflow/api/2.0/mlflow/get' do + let(:experiment_iid) { experiment.iid.to_s } + let(:route) { "/projects/#{project.id}/ml/mflow/api/2.0/mlflow/experiments/get?experiment_id=#{experiment_iid}" } + + it 'returns the experiment' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('ml/get_experiment') + expect(json_response).to include({ + 'experiment' => { + 'experiment_id' => experiment_iid, + 'name' => experiment.name, + 'lifecycle_stage' => 'active', + 'artifact_location' => 'not_implemented' + } + }) + end + + describe 'Error States' do + context 'when has access' do + context 'and experiment does not exist' do + let(:experiment_iid) { non_existing_record_iid.to_s } + + it_behaves_like 'Not Found - Resource Does Not Exist' + end + + context 'and experiment_id is not passed' do + let(:route) { "/projects/#{project.id}/ml/mflow/api/2.0/mlflow/experiments/get" } + + it_behaves_like 'Not Found - Resource Does Not Exist' + end + end + + it_behaves_like 'shared error cases' + end + end + + describe 'GET /projects/:id/ml/mflow/api/2.0/mlflow/experiments/get-by-name' do + let(:experiment_name) { experiment.name } + let(:route) do + "/projects/#{project.id}/ml/mflow/api/2.0/mlflow/experiments/get-by-name?experiment_name=#{experiment_name}" + end + + it 'returns the experiment' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('ml/get_experiment') + expect(json_response).to include({ + 'experiment' => { + 'experiment_id' => experiment.iid.to_s, + 'name' => experiment_name, + 'lifecycle_stage' => 'active', + 'artifact_location' => 'not_implemented' + } + }) + end + + describe 'Error States' do + context 'when has access but experiment does not exist' do + let(:experiment_name) { "random_experiment" } + + it_behaves_like 'Not Found - Resource Does Not Exist' + end + + context 'when has access but experiment_name is not passed' do + let(:route) { "/projects/#{project.id}/ml/mflow/api/2.0/mlflow/experiments/get-by-name" } + + it_behaves_like 'Not Found - Resource Does Not Exist' + end + + it_behaves_like 'shared error cases' + end + end + + describe 'POST /projects/:id/ml/mflow/api/2.0/mlflow/experiments/create' do + let(:route) do + "/projects/#{project.id}/ml/mflow/api/2.0/mlflow/experiments/create" + end + + let(:params) { { name: 'new_experiment' } } + let(:request) { post api(route), params: params, headers: headers } + + it 'creates the experiment' do + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to include('experiment_id' ) + end + + describe 'Error States' do + context 'when experiment name is not passed' do + let(:params) { {} } + + it_behaves_like 'Bad Request' + end + + context 'when experiment name already exists' do + let(:existing_experiment) do + create(:ml_experiments, user: current_user, project: project) + end + + let(:params) { { name: existing_experiment.name } } + + it_behaves_like 'Bad Request', 'RESOURCE_ALREADY_EXISTS' + end + + context 'when project does not exist' do + let(:route) { "/projects/#{non_existing_record_id}/ml/mflow/api/2.0/mlflow/experiments/create" } + + it_behaves_like 'Not Found', '404 Project Not Found' + end + end + end +end