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

Fixed search_pipeline spec #253

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

saimedhi
Copy link
Contributor

@saimedhi saimedhi commented Apr 18, 2024

Description

  • Corrected spec for search_pipeline.get, search_pipeline.create
  • Added search_pipeline.delete

Issues Resolved

Closes #248

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.

Copy link
Contributor

API specs implemented for 247/649 (38%) APIs.

@@ -4,14 +4,15 @@ info:
description: OpenSearch Search_pipeline API
version: 1.0.0
paths:
/_search/pipeline/{pipeline}:
/_search/pipeline/{id}:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nhtruong Do you think we should replace path param name from pipeline to id?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Both make sense to me. If the endpoints using this paths haven't made it to any clients then we're free to rename without introducing breaking changes. I know that Ruby and JS clients haven't implemented these yet. @Xtansia what about Java, Net, and Rust?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hasn't been added into any of those clients

Copy link
Collaborator

Choose a reason for hiding this comment

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

@saimedhi Then this name change is fine!

@@ -23,12 +24,26 @@ paths:
externalDocs:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nhtruong @Xtansia PUT /_search/pipeline/my_pipeline endpoint creates and updates search_pipeline.
So, should we name it search_pipeline.put instead of search_pipeline.create?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes I think so, it makes more sense semantically. Especially when things such as the _doc APIs set the precedent that "create" means fail if the item already exists.

Copy link
Collaborator

Choose a reason for hiding this comment

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

agree with @Xtansia

@nhtruong nhtruong merged commit cecb187 into opensearch-project:main Apr 18, 2024
4 checks passed
@saimedhi saimedhi deleted the fix/search_pipeline branch April 19, 2024 02:26
djmadeira added a commit to djmadeira/opensearch-api-specification that referenced this pull request May 7, 2024
additional paths

Fixed spec (opensearch-project#246)

Signed-off-by: saimedhi <[email protected]>

Fixed search_pipeline spec (opensearch-project#253)

Signed-off-by: saimedhi <[email protected]>

Updated API name: search_pipeline.create to search_pipeline.put (opensearch-project#254)

Signed-off-by: saimedhi <[email protected]>

Fixed search_pipeline.get spec (opensearch-project#255)

Signed-off-by: saimedhi <[email protected]>

Filled in Missing Defaults (opensearch-project#249)

* Filled in Missing Defaults

Signed-off-by: Theo Truong <[email protected]>

* # Wordings

Signed-off-by: Theo Truong <[email protected]>

---------

Signed-off-by: Theo Truong <[email protected]>

Replace pull_request_target workflow with two workflows that upload/download an artifact. (opensearch-project#251)

* Fix: replace pull_request_target with a download/upload artifact.

Signed-off-by: dblock <[email protected]>

* Use upload/download-artifact@v4.

Signed-off-by: dblock <[email protected]>

---------

Signed-off-by: dblock <[email protected]>

Fix: var -> const. (opensearch-project#258)

Signed-off-by: dblock <[email protected]>

Adds tools linter. (opensearch-project#260)

Signed-off-by: dblock <[email protected]>

Fix: '@typescript-eslint/indent': 'warn'. (opensearch-project#262)

Signed-off-by: dblock <[email protected]>

Removed default values from param description (opensearch-project#264)

Signed-off-by: saimedhi <[email protected]>

Generate _opendistro endpoints through merger tool (opensearch-project#257)

* Generate _opendistro endpoints through merger tool

Signed-off-by: Theo Truong <[email protected]>

* # Renamed `replaced` with `superseded`

Signed-off-by: Theo Truong <[email protected]>

* # Rebased DEVELOPER_GUIDE.md

Signed-off-by: Theo Truong <[email protected]>

* # Set Tabsize from 4 to 2

Signed-off-by: Theo Truong <[email protected]>

---------

Signed-off-by: Theo Truong <[email protected]>

Add _plugins/_notifications/channels API spec (opensearch-project#256)

* Add _plugins/_notifications/channels API

Signed-off-by: Sokratis Papadopoulos <[email protected]>

Fix obvious lints. (opensearch-project#265)

* Fix cosmetic autoformat lints.
* Fixed @typescript-eslint/no-unused-vars.
* Fixed eqeqeq.
* Fixed @typescript-eslint/consistent-type-imports.
* Fixed no-useless-return.
* Fixed @typescript-eslint/array-type.
* Rebased with changes on main.

Signed-off-by: dblock <[email protected]>

Update list notification channels url for externalDocs (opensearch-project#267)

Signed-off-by: Sokratis Papadopoulos <[email protected]>
Co-authored-by: Sokratis Papadopoulos <[email protected]>

Updated/Corrected Docs (opensearch-project#270)

* Updated/Corrected Docs

- README.md
- CLIENT_GENERATOR_GUIDE.md
- DEVELOPER_GUIDE.md
- ./tools/README.md

Signed-off-by: Theo Truong <[email protected]>

* # minor corrections

Signed-off-by: Theo Truong <[email protected]>

* # minor corrections

Signed-off-by: Theo Truong <[email protected]>

---------

Signed-off-by: Theo Truong <[email protected]>

Add lychee github action  for links checking (opensearch-project#269)

Corrected content type for bulk operations (opensearch-project#275)

* Corrected content type for bulk operations

Signed-off-by: Theo Truong <[email protected]>

* # linting

Signed-off-by: Theo Truong <[email protected]>

---------

Signed-off-by: Theo Truong <[email protected]>

Validate _superseded_operations.yaml against its JSON schema (opensearch-project#276)

* Validate _superseded_operations.yaml against its JSON schema

Signed-off-by: Theo Truong <[email protected]>

* # lint

Signed-off-by: Theo Truong <[email protected]>

* # lint

Signed-off-by: Theo Truong <[email protected]>

---------

Signed-off-by: Theo Truong <[email protected]>

Fixed Linting for Tools (opensearch-project#278)

Signed-off-by: Theo Truong <[email protected]>

Removed Root file since it's not needed anymore (opensearch-project#279)

Signed-off-by: Theo Truong <[email protected]>

Fixed missing global params (opensearch-project#280)

Signed-off-by: Theo Truong <[email protected]>

Added validation for _info.yaml (opensearch-project#281)

DRY'ed JSON schema validation logic

Signed-off-by: Theo Truong <[email protected]>

Implement inline object schema validator (opensearch-project#282)

* Implement inline object schema validator and underlying visitor pattern

Signed-off-by: Thomas Farr <[email protected]>

* Fix spec lint error

Signed-off-by: Thomas Farr <[email protected]>

---------

Signed-off-by: Thomas Farr <[email protected]>

oops

fix lint
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.

[BUG] Differences in search_pipeline API between OpenSearch Server and Specification.
3 participants