Skip to content

Commit

Permalink
Add model name to default pipeline name (#172110)
Browse files Browse the repository at this point in the history
Update the default pipeline name to include the model name, like so:

`ml-inference-<index name>-<model name>`

 Unsupported characters in the model name are mapped to `_`.
  • Loading branch information
Mikep86 authored Nov 29, 2023
1 parent 5412584 commit cca28e7
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import React, { useEffect } from 'react';
import React from 'react';

import { useValues, useActions } from 'kea';

Expand All @@ -30,7 +30,7 @@ import { IndexViewLogic } from '../../index_view_logic';
import { EMPTY_PIPELINE_CONFIGURATION, MLInferenceLogic } from './ml_inference_logic';
import { MlModelSelectOption } from './model_select_option';
import { PipelineSelectOption } from './pipeline_select_option';
import { MODEL_REDACTED_VALUE, MODEL_SELECT_PLACEHOLDER } from './utils';
import { MODEL_REDACTED_VALUE, MODEL_SELECT_PLACEHOLDER, normalizeModelName } from './utils';

const MODEL_SELECT_PLACEHOLDER_VALUE = 'model_placeholder$$';
const PIPELINE_SELECT_PLACEHOLDER_VALUE = 'pipeline_placeholder$$';
Expand Down Expand Up @@ -62,14 +62,7 @@ export const ConfigurePipeline: React.FC = () => {
const { ingestionMethod } = useValues(IndexViewLogic);
const { indexName } = useValues(IndexNameLogic);

const { existingPipeline, modelID, pipelineName } = configuration;

useEffect(() => {
setInferencePipelineConfiguration({
...configuration,
pipelineName: pipelineName || indexName,
});
}, []);
const { existingPipeline, modelID, pipelineName, isPipelineNameUserSupplied } = configuration;

const nameError = formErrors.pipelineName !== undefined && pipelineName.length > 0;

Expand Down Expand Up @@ -155,6 +148,7 @@ export const ConfigurePipeline: React.FC = () => {
onChange={(e) =>
setInferencePipelineConfiguration({
...configuration,
isPipelineNameUserSupplied: e.target.value.length > 0,
pipelineName: e.target.value,
})
}
Expand All @@ -179,6 +173,9 @@ export const ConfigurePipeline: React.FC = () => {
inferenceConfig: undefined,
modelID: value,
fieldMappings: undefined,
pipelineName: isPipelineNameUserSupplied
? pipelineName
: indexName + '-' + normalizeModelName(value),
})
}
options={modelOptions}
Expand Down Expand Up @@ -253,13 +250,10 @@ export const ConfigurePipeline: React.FC = () => {
onTabClick={(tab) => {
const isExistingPipeline = tab.id === ConfigurePipelineTabId.USE_EXISTING;
if (isExistingPipeline !== configuration.existingPipeline) {
const pipelineConfig = EMPTY_PIPELINE_CONFIGURATION;
pipelineConfig.existingPipeline = isExistingPipeline;
if (!isExistingPipeline) {
pipelineConfig.pipelineName = indexName;
}

setInferencePipelineConfiguration(pipelineConfig);
setInferencePipelineConfiguration({
...EMPTY_PIPELINE_CONFIGURATION,
existingPipeline: isExistingPipeline,
});
}
}}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { InferencePipelineInferenceConfig } from '../../../../../../../common/ty
export interface InferencePipelineConfiguration {
existingPipeline?: boolean;
inferenceConfig?: InferencePipelineInferenceConfig;
isPipelineNameUserSupplied?: boolean;
modelID: string;
pipelineName: string;
fieldMappings?: FieldMapping[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { isValidPipelineName } from './utils';
import { isValidPipelineName, normalizeModelName } from './utils';

describe('ml inference utils', () => {
describe('isValidPipelineName', () => {
Expand All @@ -24,4 +24,16 @@ describe('ml inference utils', () => {
expect(isValidPipelineName('a_pipeline_name_1"')).toEqual(false);
});
});
describe('normalizeModelName', () => {
it('no normalization', () => {
expect(normalizeModelName('valid_model_name_123')).toEqual('valid_model_name_123');
expect(normalizeModelName('valid-model-name-123')).toEqual('valid-model-name-123');
});
it('normalize model name', () => {
expect(normalizeModelName('.elser_model_2')).toEqual('_elser_model_2');
expect(normalizeModelName('model!@with#$special%^chars&*123')).toEqual(
'model__with__special__chars__123'
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { FetchPipelineResponse } from '../../../../api/pipelines/fetch_pipeline'
import { AddInferencePipelineFormErrors, InferencePipelineConfiguration } from './types';

const VALID_PIPELINE_NAME_REGEX = /^[\w\-]+$/;
const NORMALIZABLE_PIPELINE_CHARS_REGEX = /[^\w\-]/g;
export const TRAINED_MODELS_PATH = '/app/ml/trained_models';

export const isValidPipelineName = (input: string): boolean => {
Expand Down Expand Up @@ -79,6 +80,10 @@ export const validateInferencePipelineFields = (
return errors;
};

export const normalizeModelName = (modelName: string): string => {
return modelName.replace(NORMALIZABLE_PIPELINE_CHARS_REGEX, '_');
};

export const EXISTING_PIPELINE_DISABLED_MISSING_SOURCE_FIELDS = (
commaSeparatedMissingSourceFields: string
) =>
Expand Down

0 comments on commit cca28e7

Please sign in to comment.