Skip to content

Commit

Permalink
Support combining multiple output variables. (opensearch-project#737)
Browse files Browse the repository at this point in the history
* Support combining multiple output variables.

Signed-off-by: dblock <[email protected]>
Signed-off-by: Tokesh <[email protected]>
  • Loading branch information
dblock authored and Tokesh committed Dec 18, 2024
1 parent 64fc829 commit e19dc25
Show file tree
Hide file tree
Showing 12 changed files with 166 additions and 40 deletions.
1 change: 1 addition & 0 deletions .lycheeignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
https://localhost:*
http://localhost:*
http://webhook:8080
https://api.openai.com/v1/chat/completions
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
4 changes: 3 additions & 1 deletion TESTING_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 3 additions & 3 deletions tests/plugins/ml/ml/connectors.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
18 changes: 18 additions & 0 deletions tools/src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,21 @@ export function write_json (file_path: string, content: any, replacer?: (this: a
export async function sleep (ms: number): Promise<void> {
await new Promise<void>((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
}
57 changes: 57 additions & 0 deletions tools/src/tester/OutputReference.ts
Original file line number Diff line number Diff line change
@@ -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])
})
}
}
13 changes: 4 additions & 9 deletions tools/src/tester/StoryEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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
Expand Down Expand Up @@ -217,10 +218,7 @@ export default class StoryEvaluator {
static #extract_params_variables(parameters: Record<string, Parameter>, variables: Set<OutputReference>): 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))
}
})
}
Expand All @@ -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': {
Expand Down
11 changes: 4 additions & 7 deletions tools/src/tester/StoryOutputs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
17 changes: 0 additions & 17 deletions tools/src/tester/types/eval.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
25 changes: 24 additions & 1 deletion tools/tests/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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'])
})
})
})
39 changes: 39 additions & 0 deletions tools/tests/tester/OutputReference.test.ts
Original file line number Diff line number Diff line change
@@ -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].'
)
})
})
});
14 changes: 12 additions & 2 deletions tools/tests/tester/StoryOutputs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})

Expand Down Expand Up @@ -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'
})
})

Expand Down

0 comments on commit e19dc25

Please sign in to comment.