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

Add is_hidden parameter for create or update alias API #429

Merged
merged 3 commits into from
Jul 18, 2024

Conversation

gaobinlong
Copy link
Contributor

Description

In this PR, we fix the bug of create or update alias API doesn't throw exception for unsupported parameters, and also add the missing request body parameter is_hidden, so we need to update the api specification.

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.

Signed-off-by: gaobinlong <gbinlong@amazon.com>
Copy link

github-actions bot commented Jul 17, 2024

Changes Analysis

Commit SHA: f60bfdc
Comparing To SHA: 82c000f

API Changes

Summary

└─┬Paths
  ├─┬/{index}/_aliases/{name}
  │ ├─┬PUT
  │ │ └─┬Requestbody
  │ │   └─┬application/json
  │ │     └─┬Schema
  │ │       └──[➕] properties (21954:15)
  │ └─┬POST
  │   └─┬Requestbody
  │     └─┬application/json
  │       └─┬Schema
  │         └──[➕] properties (21954:15)
  └─┬/{index}/_alias/{name}
    ├─┬PUT
    │ └─┬Requestbody
    │   └─┬application/json
    │     └─┬Schema
    │       └──[➕] properties (21954:15)
    └─┬POST
      └─┬Requestbody
        └─┬application/json
          └─┬Schema
            └──[➕] properties (21954:15)

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

Report

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

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 previously requested changes Jul 17, 2024
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Thanks! See below.

So the tests for <= 2.15 pass because we weren't checking parameters? Do update them to have the correct version if there are fields used in tests against a version that doesn't support them, so we don't imply they can work.

CHANGELOG.md Outdated
@@ -46,6 +46,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Added missing fields to `/_nodes/stats` ([#415](https://github.com/opensearch-project/opensearch-api-specification/pull/415))
- Added missing metrics options to `/_nodes/stats` ([#422](https://github.com/opensearch-project/opensearch-api-specification/pull/422))
- Added tests against OpenSearch 1.3 ([#424](https://github.com/opensearch-project/opensearch-api-specification/pull/424))
- Add `is_hidden` parameter for create or update alias API
Copy link
Member

Choose a reason for hiding this comment

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

Make it look like the rest of the lines.

"Added is_hidden to /{index}/_alias/ (#...)"

x-version-added: '2.16'
description: |-
If `true`, the alias will be hidden, defaults to `false`.
type: boolean
Copy link
Member

Choose a reason for hiding this comment

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

Add default: false, and no need to say "defaults to ..." in the text. We're going to clean these up at some point.

@@ -0,0 +1,42 @@
$schema: ../../json_schemas/test_story.schema.yaml
Copy link
Member

Choose a reason for hiding this comment

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

We already have alias.yaml that has a lot of similar tests. Either add one more test for is_hidden there, or create a folder called tests/indices/alias/is_hidden.yaml and include only functionality related to this new is_hidden into it. I think the former seems simpler.

You also need to add version: '>= 2.16' into the test so it doesn't run with older OpenSearch (I expect it to fail).

@gaobinlong
Copy link
Contributor Author

Thanks @dblock , please help to take a look at the new change.

@Xtansia Xtansia dismissed dblock’s stale review July 18, 2024 03:52

Requested changes have been made

@Xtansia Xtansia merged commit b553c4b into opensearch-project:main Jul 18, 2024
11 checks passed
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.

3 participants