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.

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

Added support for application/yaml response payloads. #363

Merged
merged 4 commits into from
Jun 28, 2024

Conversation

dblock
Copy link
Member

@dblock dblock commented Jun 26, 2024

Description

On top of #360, Added application/yaml support and responses to cat APIs. This one requires deserializing the response body into an object to validate.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@dblock dblock changed the title Format yaml Added application/yaml response support. Jun 26, 2024
@dblock dblock marked this pull request as ready for review June 26, 2024 19:18
Copy link

github-actions bot commented Jun 26, 2024

Changes Analysis

Commit SHA: 2128acc
Comparing To SHA: bb63d76

API Changes

Summary


└─┬Paths
  ├─┬/_cat/health
  │ └─┬GET
  │   └─┬Responses
  │     └─┬200
  │       └──[➕] content (23178:11)
  ├─┬/_cat/indices/{index}
  │ └─┬GET
  │   └─┬Responses
  │     └─┬200
  │       └──[➕] content (23199:11)
  └─┬/_cat/indices
    └─┬GET
      └─┬Responses
        └─┬200
          └──[➕] content (23199:11)

Document Element Total Changes Breaking Changes
paths 3 0
  • Total Changes: 3
  • Additions: 3

Report

The full API changes report is available at: https://github.com/opensearch-project/opensearch-api-specification/actions/runs/9716543764/artifacts/1649566586

API Coverage

Before After Δ
Covered (%) 483 (47.31 %) 483 (47.31 %) 0 (0 %)
Uncovered (%) 538 (52.69 %) 538 (52.69 %) 0 (0 %)
Unknown 24 24 0

@dblock dblock changed the title Added application/yaml response support. Added support for application/yaml response payloads. Jun 26, 2024
@dblock dblock force-pushed the format-yaml branch 2 times, most recently from 2b2cf3f to 5e25965 Compare June 26, 2024 21:05
@@ -29,13 +29,13 @@ components:
type: string
docs.count:
description: available docs
oneOf:
anyOf:
- type: string
- nullable: true
type: string
Copy link
Collaborator

Choose a reason for hiding this comment

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

probably scope creeping but nullable is not a proper JSON schema keyword. Even if it was, this is redundant because nullable string already encapsulates non-nullable string.

In this case and a few below, we can repalce anyOf: ... with type: [string, null] according to this

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried this but it failed with some other error. Care to open an issue with some details to replace?

nhtruong
nhtruong previously approved these changes Jun 27, 2024
Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: dblock <dblock@amazon.com>
@dblock dblock dismissed nhtruong’s stale review June 27, 2024 19:05

The merge-base changed after approval.

@dblock dblock requested a review from nhtruong June 27, 2024 19:07
nhtruong
nhtruong previously approved these changes Jun 28, 2024
Signed-off-by: Theo Nam Truong <theotr@amazon.com>
@nhtruong
Copy link
Collaborator

This is stuck at "Checking for ability to merge automatically…"

@dblock dblock merged commit ed3fb4d into opensearch-project:main Jun 28, 2024
9 checks passed
@dblock dblock deleted the format-yaml branch June 28, 2024 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants