From e19dc250654887bc34642795c39317bbddffb705 Mon Sep 17 00:00:00 2001 From: "Daniel (dB.) Doubrovkine" Date: Wed, 18 Dec 2024 12:29:10 -0500 Subject: [PATCH] Support combining multiple output variables. (#737) * Support combining multiple output variables. Signed-off-by: dblock Signed-off-by: Tokesh --- .lycheeignore | 1 + CHANGELOG.md | 1 + TESTING_GUIDE.md | 4 +- tests/plugins/ml/ml/connectors.yaml | 6 +-- tools/src/helpers.ts | 18 +++++++ tools/src/tester/OutputReference.ts | 57 ++++++++++++++++++++++ tools/src/tester/StoryEvaluator.ts | 13 ++--- tools/src/tester/StoryOutputs.ts | 11 ++--- tools/src/tester/types/eval.types.ts | 17 ------- tools/tests/helpers.test.ts | 25 +++++++++- tools/tests/tester/OutputReference.test.ts | 39 +++++++++++++++ tools/tests/tester/StoryOutputs.test.ts | 14 +++++- 12 files changed, 166 insertions(+), 40 deletions(-) create mode 100644 tools/src/tester/OutputReference.ts create mode 100644 tools/tests/tester/OutputReference.test.ts diff --git a/.lycheeignore b/.lycheeignore index 1b6a61169..4c14ce320 100644 --- a/.lycheeignore +++ b/.lycheeignore @@ -1,3 +1,4 @@ https://localhost:* http://localhost:* http://webhook:8080 +https://api.openai.com/v1/chat/completions diff --git a/CHANGELOG.md b/CHANGELOG.md index 94d891a4b..213de17b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Added `_type` to `termvector` and `mtermvector` ([#734](https://github.com/opensearch-project/opensearch-api-specification/pull/734)) - Added missing `cancelled` and `resource_stats` to `/_reindex/{task_id}/_rethrottle` ([#740](https://github.com/opensearch-project/opensearch-api-specification/pull/740)) - Added missing `cancellation_time_millis` to `POST /_tasks/_cancel` ([#747](https://github.com/opensearch-project/opensearch-api-specification/pull/747)) +- Added support for combining output variables ([#737](https://github.com/opensearch-project/opensearch-api-specification/pull/737)) - Added 404 response to `/_search/scroll` ([#749](https://github.com/opensearch-project/opensearch-api-specification/pull/749)) - Added `node_failures` to `DELETE /_search/scroll` and `DELETE /_search/scroll/{scroll_id}` ([#749](https://github.com/opensearch-project/opensearch-api-specification/pull/749)) diff --git a/TESTING_GUIDE.md b/TESTING_GUIDE.md index 2812b37b2..c4590bd0a 100644 --- a/TESTING_GUIDE.md +++ b/TESTING_GUIDE.md @@ -244,7 +244,9 @@ Consider the following chapters in [ml/model_groups](tests/plugins/ml/ml/model_g ``` As you can see, the `output` field in the first chapter saves the `model_group_id` from the response body. This value is then used in the subsequent chapters to query and delete the model group. -You can also supply defaults for output values, e.g. for `payload._version` used in [cluster/routing/awareness/weights.yaml](tests/routing/cluster/routing/awareness/weights.yaml). +You can combine multiple values, e.g. `task_id: ${task.node}:${task.id}`. + +You can supply defaults for output values, e.g. for `payload._version` used in [cluster/routing/awareness/weights.yaml](tests/routing/cluster/routing/awareness/weights.yaml). ``` version: diff --git a/tests/plugins/ml/ml/connectors.yaml b/tests/plugins/ml/ml/connectors.yaml index 201019c04..c19939634 100644 --- a/tests/plugins/ml/ml/connectors.yaml +++ b/tests/plugins/ml/ml/connectors.yaml @@ -33,10 +33,10 @@ chapters: actions: - action_type: predict method: POST - url: 'https://${parameters.endpoint}/v1/chat/completions' + url: https://api.openai.com/v1/chat/completions headers: - Authorization: 'Bearer ${credential.openAI_key}' - request_body: '{ "model": "${parameters.model}", "messages": ${parameters.messages} }' + Authorization: Bearer Key + request_body: '{ "model": "model", "messages": "messages" }' response: status: 200 output: diff --git a/tools/src/helpers.ts b/tools/src/helpers.ts index 4144afd31..d338f7960 100644 --- a/tools/src/helpers.ts +++ b/tools/src/helpers.ts @@ -160,3 +160,21 @@ export function write_json (file_path: string, content: any, replacer?: (this: a export async function sleep (ms: number): Promise { await new Promise((resolve) => setTimeout(resolve, ms)) } + +/** + * Returns a string split using a delimiter, but only up to a certain number of times, + * returning the remainder of the string as the last element if the result. + * + * @param str a string + * @param delim delimiter + * @param count max number of splits + * @returns an array of strings + */ +export function split(str: any, delim: string, count: number = 0): string[] { + if (str === undefined) return [] + const parts = str.split(delim) + if (count <= 0 || parts.length <= count) return parts + const result = parts.slice(0, count - 1) + result.push(parts.slice(count - 1).join(delim)) + return result +} diff --git a/tools/src/tester/OutputReference.ts b/tools/src/tester/OutputReference.ts new file mode 100644 index 000000000..a76fb6249 --- /dev/null +++ b/tools/src/tester/OutputReference.ts @@ -0,0 +1,57 @@ +/* +* Copyright OpenSearch Contributors +* SPDX-License-Identifier: Apache-2.0 +* +* The OpenSearch Contributors require contributions made to +* this file be licensed under the Apache-2.0 license or a +* compatible open source license. +*/ + +import { split } from "../helpers" + +export class OutputReference { + chapter_id: string + output_name: string + + constructor(chapter_id: string, output_name: string) { + this.chapter_id = chapter_id + this.output_name = output_name + } + + /** + * Parses a string and returns a collection of output references that it may contain. + * + * @param str a string + * @returns an array of output references + */ + static parse(str: string): OutputReference[] { + const pattern = /\$\{([^}]+)\}/g + let match + var result = [] + while ((match = pattern.exec(str)) !== null) { + const spl = split(match[1], '.', 2) + result.push(new OutputReference(spl[0], spl[1])) + } + return result + } + + /** + * Replaces occurrences of output references. + * Takes special care of preserving the type of the reference value if the entire string is a reference. + * + * @param str a string that may contain output references + * @param callback a callback for fetching reference values + * @returns a string with output references replaced by their values + */ + static replace(str: string, callback: (chapter_id: any, variable: any) => string): any { + let full_match = str.match(/^\$\{([^}]+)\}$/) + if (full_match) { + // the entire string is a reference, preserve the type of the returned value + const spl = split(full_match[1], '.', 2) + return callback(spl[0], spl[1]) + } else return str.replace(/\$\{([^}]+)\}/g, (_, k) => { + const spl = split(k, '.', 2) + return callback(spl[0], spl[1]) + }) + } +} diff --git a/tools/src/tester/StoryEvaluator.ts b/tools/src/tester/StoryEvaluator.ts index a17df942f..eb0bbf2f7 100644 --- a/tools/src/tester/StoryEvaluator.ts +++ b/tools/src/tester/StoryEvaluator.ts @@ -8,7 +8,7 @@ */ import { ChapterRequest, Parameter, SupplementalChapter } from './types/story.types' -import { type StoryFile, type ChapterEvaluation, Result, type StoryEvaluation, OutputReference } from './types/eval.types' +import { type StoryFile, type ChapterEvaluation, Result, type StoryEvaluation } from './types/eval.types' import type ChapterEvaluator from './ChapterEvaluator' import { overall_result } from './helpers' import { StoryOutputs } from './StoryOutputs' @@ -17,6 +17,7 @@ import { ChapterOutput } from './ChapterOutput' import * as semver from '../_utils/semver' import _ from 'lodash' import { ParsedChapter, ParsedStory } from './types/parsed_story.types' +import { OutputReference } from './OutputReference' export default class StoryEvaluator { private readonly _chapter_evaluator: ChapterEvaluator @@ -217,10 +218,7 @@ export default class StoryEvaluator { static #extract_params_variables(parameters: Record, variables: Set): void { Object.values(parameters ?? {}).forEach((param) => { if (typeof param === 'string') { - const ref = OutputReference.parse(param) - if (ref) { - variables.add(ref) - } + OutputReference.parse(param).forEach((ref) => variables.add(ref)) } }) } @@ -229,10 +227,7 @@ export default class StoryEvaluator { const request_type = typeof request switch (request_type) { case 'string': { - const ref = OutputReference.parse(request as string) - if (ref !== undefined) { - variables.add(ref) - } + OutputReference.parse(request as string).forEach((ref) => variables.add(ref)) break } case 'object': { diff --git a/tools/src/tester/StoryOutputs.ts b/tools/src/tester/StoryOutputs.ts index 41b8d9e71..b535bc62e 100644 --- a/tools/src/tester/StoryOutputs.ts +++ b/tools/src/tester/StoryOutputs.ts @@ -8,7 +8,7 @@ */ import { ChapterOutput } from './ChapterOutput' -import { OutputReference } from './types/eval.types' +import { OutputReference } from './OutputReference' import { type Parameter } from './types/story.types' export class StoryOutputs { @@ -48,12 +48,9 @@ export class StoryOutputs { } resolve_string (str: string): any { - const ref = OutputReference.parse(str) - if (ref) { - return this.get_output_value(ref.chapter_id, ref.output_name) - } else { - return str - } + return OutputReference.replace(str, (chapter_id, output_name) => { + return this.get_output_value(chapter_id as string, output_name as string) + }) } resolve_value (payload: any): any { diff --git a/tools/src/tester/types/eval.types.ts b/tools/src/tester/types/eval.types.ts index e65b80c3c..0686a6896 100644 --- a/tools/src/tester/types/eval.types.ts +++ b/tools/src/tester/types/eval.types.ts @@ -83,20 +83,3 @@ export enum Result { SKIPPED = 'SKIPPED', ERROR = 'ERROR', } - -export class OutputReference { - chapter_id: string - output_name: string - private constructor (chapter_id: string, output_name: string) { - this.chapter_id = chapter_id - this.output_name = output_name - } - - static parse (str: string): OutputReference | undefined { - if (str.startsWith('${') && str.endsWith('}')) { - const spl = str.slice(2, -1).split('.', 2) - return { chapter_id: spl[0], output_name: spl[1] } - } - return undefined - } -} diff --git a/tools/tests/helpers.test.ts b/tools/tests/helpers.test.ts index f91dde573..3e1f15476 100644 --- a/tools/tests/helpers.test.ts +++ b/tools/tests/helpers.test.ts @@ -8,7 +8,7 @@ */ import _ from 'lodash' -import { delete_matching_keys, find_refs, sort_array_by_keys, to_json, to_ndjson } from '../src/helpers' +import { delete_matching_keys, find_refs, sort_array_by_keys, to_json, to_ndjson, split } from '../src/helpers' describe('helpers', () => { describe('sort_array_by_keys', () => { @@ -195,4 +195,27 @@ describe('helpers', () => { })).toEqual(new Set(['#1', '#2', '#3', '#dup', '#/schemas/schema1', '#/schemas/schema2'])) }) }) + + describe('split', () => { + test('undefined', () => { + expect(split(undefined, '.')).toEqual([]) + }) + + test('one element', () => { + expect(split('str', '.')).toEqual(['str']) + expect(split('str', '.', -1)).toEqual(['str']) + expect(split('str', '.', 0)).toEqual(['str']) + expect(split('str', '.', 10)).toEqual(['str']) + }) + + test('multiple elements', () => { + expect(split('x.y.z', '.')).toEqual(['x', 'y', 'z']) + expect(split('x.y.z', '.', -1)).toEqual(['x', 'y', 'z']) + expect(split('x.y.z', '.', 0)).toEqual(['x', 'y', 'z']) + expect(split('x.y.z', '.', 1)).toEqual(['x.y.z']) + expect(split('x.y.z', '.', 2)).toEqual(['x', 'y.z']) + expect(split('x.y.z', '.', 3)).toEqual(['x', 'y', 'z']) + expect(split('x.y.z', '.', 4)).toEqual(['x', 'y', 'z']) + }) + }) }) diff --git a/tools/tests/tester/OutputReference.test.ts b/tools/tests/tester/OutputReference.test.ts new file mode 100644 index 000000000..76aa4a067 --- /dev/null +++ b/tools/tests/tester/OutputReference.test.ts @@ -0,0 +1,39 @@ +/* +* Copyright OpenSearch Contributors +* SPDX-License-Identifier: Apache-2.0 +* +* The OpenSearch Contributors require contributions made to +* this file be licensed under the Apache-2.0 license or a +* compatible open source license. +*/ + +import { OutputReference } from "tester/OutputReference"; + +describe('OutputReference', () => { + let f = (id: any, k: any): string => `[${id}:${k}]` + + describe('parse', () => { + it('replaces', () => { + expect(OutputReference.parse('string')).toEqual([]) + expect(OutputReference.parse('${k.v}')).toEqual([new OutputReference('k', 'v')]) + expect(OutputReference.parse('${k.value}')).toEqual([new OutputReference('k', 'value')]) + expect(OutputReference.parse('${k.v.m}')).toEqual([new OutputReference('k', 'v.m')]) + expect(OutputReference.parse('A reference to ${k.v.m} and ${x.y}.')).toEqual([ + new OutputReference('k', 'v.m'), + new OutputReference('x', 'y') + ]) + }) + }) + + describe('replace', () => { + it('replaces', () => { + expect(OutputReference.replace('string', f)).toEqual('string') + expect(OutputReference.replace('${k.v}', f)).toEqual('[k:v]') + expect(OutputReference.replace('${k.value}', f)).toEqual('[k:value]') + expect(OutputReference.replace('${k.v.m}', f)).toEqual('[k:v.m]') + expect(OutputReference.replace('A reference to ${k.v.m} and ${x} and ${x.y}.', f)).toEqual( + 'A reference to [k:v.m] and [x:undefined] and [x:y].' + ) + }) + }) +}); diff --git a/tools/tests/tester/StoryOutputs.test.ts b/tools/tests/tester/StoryOutputs.test.ts index 3017b7a66..9339a860b 100644 --- a/tools/tests/tester/StoryOutputs.test.ts +++ b/tools/tests/tester/StoryOutputs.test.ts @@ -19,7 +19,11 @@ const story_outputs = new StoryOutputs({ test('resolve_string', () => { expect(story_outputs.resolve_string('${chapter_id.x}')).toEqual(1) + expect(story_outputs.resolve_string('${chapter_id.y}')).toEqual(2) expect(story_outputs.resolve_string('${invalid_id.x}')).toBeUndefined() + expect(story_outputs.resolve_string('${invalid_id.y}')).toBeUndefined() + expect(story_outputs.resolve_string('${chapter_id.x} and ${chapter_id.y}')).toEqual('1 and 2') + expect(story_outputs.resolve_string('${chapter_id.x} and ${chapter_id.y} and ${invalid.invalid}')).toEqual('1 and 2 and undefined') expect(story_outputs.resolve_string('some_str')).toEqual('some_str') }) @@ -55,13 +59,19 @@ test('resolve_params', () => { a: '${chapter_id.x}', b: '${chapter_id.y}', c: 3, - d: 'str' + d: 'str', + e: '${chapter_id.x}:${chapter_id.y}', + f: 'x=${chapter_id.x}', + g: '${chapter_id.y}=y' } expect(story_outputs.resolve_params(parameters)).toEqual({ a: 1, b: 2, c: 3, - d: 'str' + d: 'str', + e: '1:2', + f: 'x=1', + g: '2=y' }) })