Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add additionalProperties to ErrorCause and fix creation_date type for list_dangling_indices #462

Merged
merged 1 commit into from
Aug 5, 2024
Merged

Add additionalProperties to ErrorCause and fix creation_date type for list_dangling_indices #462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Added `/_plugins/_ppl`, `explain` and `stats` ([#460](https://github.com/opensearch-project/opensearch-api-specification/pull/460))
- Added tests against OpenSearch 3.0 ([#459](https://github.com/opensearch-project/opensearch-api-specification/pull/459))
- Added support for request headers in tests [#461](https://github.com/opensearch-project/opensearch-api-specification/pull/461)

- Added metadata additionalProperties to `ErrorCause` ([#462](https://github.com/opensearch-project/opensearch-api-specification/pull/462))
- Added `creation_date` field to `DanglingIndex` ([#462](https://github.com/opensearch-project/opensearch-api-specification/pull/462))

### Changed

- Replaced Smithy with a native OpenAPI spec ([#189](https://github.com/opensearch-project/opensearch-api-specification/issues/189))
Expand Down
4 changes: 4 additions & 0 deletions spec/schemas/_common.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ components:
UnitMillis:
description: Time unit for milliseconds.
type: number
format: int64
DurationLarge:
description: |-
A date histogram interval. Similar to `Duration` with additional units: `w` (week), `M` (month), `q` (quarter) and `y` (year).
Expand Down Expand Up @@ -314,6 +315,9 @@ components:
$ref: '#/components/schemas/ErrorCause'
required:
- type
additionalProperties:
title: metadata
description: Additional details about the error.
DurationValueUnitNanos:
allOf:
- $ref: '#/components/schemas/UnitNanos'
Expand Down
2 changes: 2 additions & 0 deletions spec/schemas/dangling_indices.list_dangling_indices.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ components:
type: string
index_uuid:
type: string
creation_date:
$ref: '_common.yaml#/components/schemas/DateTime'
Comment on lines +16 to +17
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking we might want some way of annotating properties as being the human readable variant of another property and dependent on ?human. Would be useful for documentation or want to do some custom logic in clients/generators.

Maybe something like:

creation_date:
  x-human-readable-of: creation_date_millis

Thoughts @dblock @nhtruong?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does feel a little too special, applying only to the _cat API. Are there exceptions where something like x always have a pair of x_millis and lives only under _cat?

Copy link
Collaborator Author

@Xtansia Xtansia Aug 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well it's not only the _cat api as this particular one is in the dangling_indices api and not only applies to _millis variants, also bytes ones

https://github.com/search?q=repo%3Aopensearch-project%2FOpenSearch%20humanReadableField&type=code

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be over-engineering to hide it under a type?

total:
   type: ...HumanlyReadableByteSize
   description: Total number of bytes.

would get automatically expanded as total and total_in_bytes when spec gets generated?

I'm just thinking out loud. The drawback of x-human-readable-of is that it's a relationship that needs to be validated. If you think it's the cleanest way to do it, go for it - and maybe @nhtruong has some other ideas?

creation_date_millis:
$ref: '_common.yaml#/components/schemas/EpochTimeUnitMillis'
node_ids:
Expand Down
2 changes: 1 addition & 1 deletion tools/src/_utils/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export function determine_possible_schema_types (doc: OpenAPIV3.Document, schema
if (schema?.anyOf !== undefined) return collect_all(schema.anyOf)
if (schema?.oneOf !== undefined) return collect_all(schema.oneOf)

if (schema == null || Object.keys(schema).filter(k => k !== 'description').length == 0) return SCHEMA_OBJECT_TYPES
if (schema == null || Object.keys(schema).filter(k => k !== 'description' && k !== 'title').length == 0) return SCHEMA_OBJECT_TYPES
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please explain this change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If nothing is specified in the schema other than "informational" properties (ie. description or title) than it can be any type.


throw new Error(`Unable to determine possible types of schema: ${to_json(schema)}`)
}
Expand Down
Loading