From 2c2aa104adb2183b432301f8f4ec386fc5fd73af Mon Sep 17 00:00:00 2001 From: Thomas Farr Date: Thu, 22 Aug 2024 11:24:49 +1200 Subject: [PATCH 1/5] Fix tasks namespace Signed-off-by: Thomas Farr --- CHANGELOG.md | 1 + DEVELOPER_GUIDE.md | 1 + spec/namespaces/_core.yaml | 126 ++++---------------------- spec/namespaces/tasks.yaml | 2 +- spec/schemas/_common.yaml | 151 +++++++++++++++++++++++++++----- spec/schemas/tasks._common.yaml | 81 ++++++++++------- 6 files changed, 197 insertions(+), 165 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c48b84eac..9398201a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -150,6 +150,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Fixed missing fields in `_cat` API ([#551](https://github.com/opensearch-project/opensearch-api-specification/pull/551)) - Fixed `geo_distance` query spec ([#561](https://github.com/opensearch-project/opensearch-api-specification/pull/561)) - Fixed `geo_bounding_box` and `geo_shape` queries ([#531](https://github.com/opensearch-project/opensearch-api-specification/pull/531)) +- Fixed tasks namespace schemas ([#520](https://github.com/opensearch-project/opensearch-api-specification/pull/520)) ### Security diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 43c30d7f3..e454d638d 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -146,6 +146,7 @@ This repository includes several OpenAPI Specification Extensions to fill in any - `x-ignorable`: Denotes that the operation should be ignored by the client generator. This is used in operation groups where some operations have been replaced by newer ones, but we still keep them in the specs because the server still supports them. - `x-global`: Denotes that the parameter is a global parameter that is included in every operation. These parameters are listed in the [spec/_global_parameters.yaml](spec/_global_parameters.yaml). - `x-default`: Contains the default value of a parameter. This is often used to override the default value specified in the schema, or to avoid accidentally changing the default value when updating a shared schema. +- `x-deprecated-enums`: Contains the values of an enum which should be annotated as deprecated in the client. - `x-distributions-included`: Contains a list of distributions known to include the API. - `x-distributions-excluded`: Contains a list of distributions known to exclude the API. diff --git a/spec/namespaces/_core.yaml b/spec/namespaces/_core.yaml index e4eae5c8f..c9d5927be 100644 --- a/spec/namespaces/_core.yaml +++ b/spec/namespaces/_core.yaml @@ -2719,42 +2719,12 @@ components: content: application/json: schema: - type: object - properties: - batches: - type: number - deleted: - type: number - failures: - type: array - items: - $ref: '../schemas/_common.yaml#/components/schemas/BulkIndexByScrollFailure' - noops: - type: number - requests_per_second: - type: number - retries: - $ref: '../schemas/_common.yaml#/components/schemas/Retries' - slice_id: - type: number - task: - $ref: '../schemas/_common.yaml#/components/schemas/TaskId' - throttled: - $ref: '../schemas/_common.yaml#/components/schemas/Duration' - throttled_millis: - $ref: '../schemas/_common.yaml#/components/schemas/DurationValueUnitMillis' - throttled_until: - $ref: '../schemas/_common.yaml#/components/schemas/Duration' - throttled_until_millis: - $ref: '../schemas/_common.yaml#/components/schemas/DurationValueUnitMillis' - timed_out: - type: boolean - took: - $ref: '../schemas/_common.yaml#/components/schemas/DurationValueUnitMillis' - total: - type: number - version_conflicts: - type: number + oneOf: + - $ref: '../schemas/_common.yaml#/components/schemas/BulkByScrollResponseBase' + - type: object + properties: + task: + $ref: '../schemas/_common.yaml#/components/schemas/TaskId' delete_by_query_rethrottle@200: content: application/json: @@ -2982,42 +2952,12 @@ components: content: application/json: schema: - type: object - properties: - batches: - type: number - created: - type: number - deleted: - type: number - failures: - type: array - items: - $ref: '../schemas/_common.yaml#/components/schemas/BulkIndexByScrollFailure' - noops: - type: number - retries: - $ref: '../schemas/_common.yaml#/components/schemas/Retries' - requests_per_second: - type: number - slice_id: - type: number - task: - $ref: '../schemas/_common.yaml#/components/schemas/TaskId' - throttled_millis: - $ref: '../schemas/_common.yaml#/components/schemas/EpochTimeUnitMillis' - throttled_until_millis: - $ref: '../schemas/_common.yaml#/components/schemas/EpochTimeUnitMillis' - timed_out: - type: boolean - took: - $ref: '../schemas/_common.yaml#/components/schemas/DurationValueUnitMillis' - total: - type: number - updated: - type: number - version_conflicts: - type: number + oneOf: + - $ref: '../schemas/_common.yaml#/components/schemas/BulkByScrollResponseBase' + - type: object + properties: + task: + $ref: '../schemas/_common.yaml#/components/schemas/TaskId' reindex_rethrottle@200: content: application/json: @@ -3178,42 +3118,12 @@ components: content: application/json: schema: - type: object - properties: - batches: - type: number - failures: - type: array - items: - $ref: '../schemas/_common.yaml#/components/schemas/BulkIndexByScrollFailure' - noops: - type: number - deleted: - type: number - requests_per_second: - type: number - retries: - $ref: '../schemas/_common.yaml#/components/schemas/Retries' - task: - $ref: '../schemas/_common.yaml#/components/schemas/TaskId' - timed_out: - type: boolean - took: - $ref: '../schemas/_common.yaml#/components/schemas/DurationValueUnitMillis' - total: - type: number - updated: - type: number - version_conflicts: - type: number - throttled: - $ref: '../schemas/_common.yaml#/components/schemas/Duration' - throttled_millis: - $ref: '../schemas/_common.yaml#/components/schemas/DurationValueUnitMillis' - throttled_until: - $ref: '../schemas/_common.yaml#/components/schemas/Duration' - throttled_until_millis: - $ref: '../schemas/_common.yaml#/components/schemas/DurationValueUnitMillis' + oneOf: + - $ref: '../schemas/_common.yaml#/components/schemas/BulkByScrollResponseBase' + - type: object + properties: + task: + $ref: '../schemas/_common.yaml#/components/schemas/TaskId' update_by_query_rethrottle@200: content: application/json: diff --git a/spec/namespaces/tasks.yaml b/spec/namespaces/tasks.yaml index 013155f8f..a0409b8a6 100644 --- a/spec/namespaces/tasks.yaml +++ b/spec/namespaces/tasks.yaml @@ -90,7 +90,7 @@ components: task: $ref: '../schemas/tasks._common.yaml#/components/schemas/TaskInfo' response: - type: object + $ref: '../schemas/tasks._common.yaml#/components/schemas/TaskResponse' error: $ref: '../schemas/_common.yaml#/components/schemas/ErrorCause' required: diff --git a/spec/schemas/_common.yaml b/spec/schemas/_common.yaml index 3c7c8c6aa..7cf64b9ae 100644 --- a/spec/schemas/_common.yaml +++ b/spec/schemas/_common.yaml @@ -1415,25 +1415,6 @@ components: type: string enum: - auto - BulkIndexByScrollFailure: - type: object - properties: - cause: - $ref: '#/components/schemas/ErrorCause' - id: - $ref: '#/components/schemas/Id' - index: - $ref: '#/components/schemas/IndexName' - status: - type: number - type: - type: string - required: - - cause - - id - - index - - status - - type Retries: type: object properties: @@ -1960,6 +1941,8 @@ components: - remote_cluster_client - transform - voting_only + x-deprecated-enums: + - master HttpHeaders: type: object additionalProperties: @@ -1990,11 +1973,7 @@ components: transport_address: $ref: '#/components/schemas/TransportAddress' required: - - attributes - - host - - ip - name - - transport_address RankContainer: type: object properties: @@ -2111,3 +2090,129 @@ components: - expand - fetch - query + BulkByScrollTaskStatus: + type: object + properties: + slice_id: + type: integer + format: int32 + total: + description: The number of documents that were successfully processed. + type: integer + format: int64 + updated: + description: The number of documents that were successfully updated, for example, a document with same ID already existed prior to reindex updating it. + type: integer + format: int64 + created: + description: The number of documents that were successfully created. + type: integer + format: int64 + deleted: + description: The number of documents that were successfully deleted. + type: integer + format: int64 + batches: + description: The number of scroll responses pulled back by the reindex. + type: integer + format: int32 + version_conflicts: + description: The number of version conflicts that reindex hits. + type: integer + format: int64 + noops: + description: The number of documents that were ignored. + type: integer + format: int64 + retries: + $ref: '#/components/schemas/Retries' + throttled_millis: + $ref: '#/components/schemas/DurationValueUnitMillis' + throttled: + $ref: '#/components/schemas/Duration' + requests_per_second: + description: The number of requests per second effectively executed during the reindex. + type: number + format: float + canceled: + type: string + throttled_until_millis: + $ref: '#/components/schemas/DurationValueUnitMillis' + throttled_until: + $ref: '#/components/schemas/Duration' + slices: + type: array + items: + $ref: '#/components/schemas/BulkByScrollTaskStatusOrException' + required: + - batches + - deleted + - noops + - requests_per_second + - retries + - throttled_millis + - throttled_until_millis + - total + - version_conflicts + BulkByScrollTaskStatusOrException: + oneOf: + - title: status + $ref: '#/components/schemas/BulkByScrollTaskStatus' + - title: exception + $ref: '#/components/schemas/ErrorCause' + BulkByScrollResponseBase: + allOf: + - $ref: '#/components/schemas/BulkByScrollTaskStatus' + - type: object + properties: + took: + type: integer + format: int64 + timed_out: + type: boolean + failures: + type: array + items: + $ref: '#/components/schemas/BulkByScrollFailure' + required: + - failures + - timed_out + - took + BulkByScrollFailure: + anyOf: + - $ref: '#/components/schemas/BulkItemResponseFailure' + - $ref: '#/components/schemas/ScrollableHitSourceSearchFailure' + BulkItemResponseFailure: + type: object + properties: + cause: + $ref: '#/components/schemas/ErrorCause' + id: + $ref: '#/components/schemas/Id' + index: + $ref: '#/components/schemas/IndexName' + status: + type: integer + format: int32 + required: + - cause + - index + - status + ScrollableHitSourceSearchFailure: + type: object + properties: + index: + $ref: '#/components/schemas/IndexName' + shard: + type: integer + format: int32 + node: + type: string + status: + type: integer + format: int32 + reason: + $ref: '#/components/schemas/ErrorCause' + required: + - reason + - status diff --git a/spec/schemas/tasks._common.yaml b/spec/schemas/tasks._common.yaml index 8e5399cb4..c5afaccda 100644 --- a/spec/schemas/tasks._common.yaml +++ b/spec/schemas/tasks._common.yaml @@ -21,34 +21,20 @@ components: description: Task information grouped by node, if `group_by` was set to `node` (the default). type: object additionalProperties: - $ref: '#/components/schemas/NodeTasks' + $ref: '#/components/schemas/TaskExecutingNode' tasks: $ref: '#/components/schemas/TaskInfos' - NodeTasks: - type: object - properties: - name: - $ref: '_common.yaml#/components/schemas/NodeId' - transport_address: - $ref: '_common.yaml#/components/schemas/TransportAddress' - host: - $ref: '_common.yaml#/components/schemas/Host' - ip: - $ref: '_common.yaml#/components/schemas/Ip' - roles: - type: array - items: - type: string - attributes: - type: object - additionalProperties: - type: string - tasks: - type: object - additionalProperties: - $ref: '#/components/schemas/TaskInfo' - required: - - tasks + TaskExecutingNode: + allOf: + - $ref: '_common.yaml#/components/schemas/BaseNode' + - type: object + properties: + tasks: + type: object + additionalProperties: + $ref: '#/components/schemas/TaskInfo' + required: + - tasks TaskInfo: type: object properties: @@ -76,8 +62,7 @@ components: start_time_in_millis: $ref: '_common.yaml#/components/schemas/EpochTimeUnitMillis' status: - description: Task status information can vary wildly from task to task. - type: object + $ref: '#/components/schemas/Status' type: type: string parent_task_id: @@ -93,13 +78,15 @@ components: - type TaskInfos: oneOf: - - type: array + - title: grouped_by_none + type: array items: $ref: '#/components/schemas/TaskInfo' - - type: object + - title: grouped_by_parents + type: object additionalProperties: - $ref: '#/components/schemas/ParentTaskInfo' - ParentTaskInfo: + $ref: '#/components/schemas/TaskGroup' + TaskGroup: allOf: - $ref: '#/components/schemas/TaskInfo' - type: object @@ -107,10 +94,38 @@ components: children: type: array items: - $ref: '#/components/schemas/TaskInfo' + $ref: '#/components/schemas/TaskGroup' GroupBy: type: string enum: - nodes - none - parents + Status: + description: Task status information can vary wildly from task to task. + anyOf: + - $ref: '#/components/schemas/ReplicationTaskStatus' + - $ref: '_common.yaml#/components/schemas/BulkByScrollTaskStatus' + - $ref: '#/components/schemas/PersistentTaskStatus' + - $ref: '#/components/schemas/RawTaskStatus' + ReplicationTaskStatus: + type: object + properties: + phase: + type: string + required: + - phase + PersistentTaskStatus: + type: object + properties: + state: + type: string + required: + - state + RawTaskStatus: + type: object + additionalProperties: + title: metadata + TaskResponse: + anyOf: + - $ref: '_common.yaml#/components/schemas/BulkByScrollResponseBase' From 624861995ac94a7ea3dae78671af63a9cf27a61b Mon Sep 17 00:00:00 2001 From: Thomas Farr Date: Tue, 10 Sep 2024 09:36:04 +1200 Subject: [PATCH 2/5] Adjust mechanism for deprecating enum values Signed-off-by: Thomas Farr --- DEVELOPER_GUIDE.md | 1 - spec/schemas/_common.yaml | 45 ++++++++++++++++++++--------------- tests/default/tasks/list.yaml | 25 +++++++++++++++++++ 3 files changed, 51 insertions(+), 20 deletions(-) create mode 100644 tests/default/tasks/list.yaml diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index e454d638d..43c30d7f3 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -146,7 +146,6 @@ This repository includes several OpenAPI Specification Extensions to fill in any - `x-ignorable`: Denotes that the operation should be ignored by the client generator. This is used in operation groups where some operations have been replaced by newer ones, but we still keep them in the specs because the server still supports them. - `x-global`: Denotes that the parameter is a global parameter that is included in every operation. These parameters are listed in the [spec/_global_parameters.yaml](spec/_global_parameters.yaml). - `x-default`: Contains the default value of a parameter. This is often used to override the default value specified in the schema, or to avoid accidentally changing the default value when updating a shared schema. -- `x-deprecated-enums`: Contains the values of an enum which should be annotated as deprecated in the client. - `x-distributions-included`: Contains a list of distributions known to include the API. - `x-distributions-excluded`: Contains a list of distributions known to exclude the API. diff --git a/spec/schemas/_common.yaml b/spec/schemas/_common.yaml index 7cf64b9ae..95a3f523c 100644 --- a/spec/schemas/_common.yaml +++ b/spec/schemas/_common.yaml @@ -1924,25 +1924,32 @@ components: items: $ref: '#/components/schemas/NodeRole' NodeRole: - type: string - enum: - - client - - cluster_manager - - coordinating_only - - data - - data_cold - - data_content - - data_frozen - - data_hot - - data_warm - - ingest - - master - - ml - - remote_cluster_client - - transform - - voting_only - x-deprecated-enums: - - master + oneOf: + - type: string + enum: + - client + - coordinating_only + - data + - data_cold + - data_content + - data_frozen + - data_hot + - data_warm + - ingest + - ml + - remote_cluster_client + - transform + - voting_only + - type: string + enum: + - master + deprecated: true + x-version-deprecated: '2.0' + x-deprecation-message: To promote inclusive language, use 'cluster_manager' instead. + - type: string + enum: + - cluster_manager + x-version-added: '2.0' HttpHeaders: type: object additionalProperties: diff --git a/tests/default/tasks/list.yaml b/tests/default/tasks/list.yaml new file mode 100644 index 000000000..2924942c9 --- /dev/null +++ b/tests/default/tasks/list.yaml @@ -0,0 +1,25 @@ +$schema: ../../../json_schemas/test_story.schema.yaml + +description: Test tasks list endpoint. +chapters: + - synopsis: List tasks grouped by node. + path: /_tasks + method: GET + parameters: + group_by: node + response: + status: 200 + - synopsis: List tasks grouped by parent. + path: /_tasks + method: GET + parameters: + group_by: parent + response: + status: 200 + - synopsis: List tasks grouped by none. + path: /_tasks + method: GET + parameters: + group_by: none + response: + status: 200 \ No newline at end of file From 5be7347b4751157a97569a221d1629826723e8b3 Mon Sep 17 00:00:00 2001 From: Thomas Farr Date: Tue, 10 Sep 2024 09:57:31 +1200 Subject: [PATCH 3/5] More verbose error Signed-off-by: Thomas Farr --- tools/src/_utils/JsonSchemaValidator.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/src/_utils/JsonSchemaValidator.ts b/tools/src/_utils/JsonSchemaValidator.ts index ff33dbc1f..426241689 100644 --- a/tools/src/_utils/JsonSchemaValidator.ts +++ b/tools/src/_utils/JsonSchemaValidator.ts @@ -38,7 +38,13 @@ export default class JsonSchemaValidator { addFormats(this.ajv); if (options.ajv_errors_opts != null) ajv_errors(this.ajv, options.ajv_errors_opts) for (const keyword of options.additional_keywords ?? []) this.ajv.addKeyword(keyword) - Object.entries(options.reference_schemas ?? {}).forEach(([key, schema]) => this.ajv.addSchema(schema, key)) + Object.entries(options.reference_schemas ?? {}).forEach(([key, schema]) => { + try { + this.ajv.addSchema(schema, key); + } catch (e) { + throw new Error(`Failed to add schema ${key}: \`${JSON.stringify(schema)}\``, { cause: e }) + } + }) this.errors_parser = new AjvErrorsParser(this.ajv, options.errors_text_opts) if (default_schema) this._validate = this.ajv.compile(default_schema) } From f322d7016fc5cc8aa48b8806f02257c5223d8b5b Mon Sep 17 00:00:00 2001 From: Thomas Farr Date: Tue, 10 Sep 2024 10:00:37 +1200 Subject: [PATCH 4/5] Fix group_by Signed-off-by: Thomas Farr --- tests/default/tasks/list.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/default/tasks/list.yaml b/tests/default/tasks/list.yaml index 2924942c9..a687e170c 100644 --- a/tests/default/tasks/list.yaml +++ b/tests/default/tasks/list.yaml @@ -6,14 +6,14 @@ chapters: path: /_tasks method: GET parameters: - group_by: node + group_by: nodes response: status: 200 - synopsis: List tasks grouped by parent. path: /_tasks method: GET parameters: - group_by: parent + group_by: parents response: status: 200 - synopsis: List tasks grouped by none. From dec5cc67929fd40547d68e78010f1223d18ed550 Mon Sep 17 00:00:00 2001 From: Thomas Farr Date: Tue, 10 Sep 2024 11:26:16 +1200 Subject: [PATCH 5/5] Add test Signed-off-by: Thomas Farr --- tools/src/_utils/JsonSchemaValidator.ts | 2 +- tools/tests/_utils/JsonSchemaValidator.test.ts | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/tools/src/_utils/JsonSchemaValidator.ts b/tools/src/_utils/JsonSchemaValidator.ts index 426241689..850468363 100644 --- a/tools/src/_utils/JsonSchemaValidator.ts +++ b/tools/src/_utils/JsonSchemaValidator.ts @@ -42,7 +42,7 @@ export default class JsonSchemaValidator { try { this.ajv.addSchema(schema, key); } catch (e) { - throw new Error(`Failed to add schema ${key}: \`${JSON.stringify(schema)}\``, { cause: e }) + throw new Error(`Failed to add schema ${key} (${JSON.stringify(schema)}):\n\t${e instanceof Error ? e.message : e as string}`) } }) this.errors_parser = new AjvErrorsParser(this.ajv, options.errors_text_opts) diff --git a/tools/tests/_utils/JsonSchemaValidator.test.ts b/tools/tests/_utils/JsonSchemaValidator.test.ts index 474dc1206..83b7c29a8 100644 --- a/tools/tests/_utils/JsonSchemaValidator.test.ts +++ b/tools/tests/_utils/JsonSchemaValidator.test.ts @@ -83,4 +83,17 @@ describe('JsonSchemaValidator', () => { expect(validator.validate_schema(invalid_schema)).toEqual('data/required must be array'); }); + + test('constructing with invalid reference schema throws descriptive error', () => { + const invalid_options = { + reference_schemas: { + '#/components/schemas/Invalid': { + type: 'string', + required: true, // required must be an array + } + } + } + expect(() => new JsonSchemaValidator(schema, invalid_options)) + .toThrow('Failed to add schema #/components/schemas/Invalid ({"type":"string","required":true}):\n\tschema is invalid: data/required must be array'); + }); }); \ No newline at end of file