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 Specs for Search Pipeline SearchResponseProcessors #437

Open
dbwiddis opened this issue Jul 18, 2024 · 2 comments
Open

Add Specs for Search Pipeline SearchResponseProcessors #437

dbwiddis opened this issue Jul 18, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@dbwiddis
Copy link
Member

What is the bug?

Coming from opensearch-project/OpenSearch#14800 (comment)

I need to add specs/test for two new search response processors. However, non of the existing processors exist to serve as an example. A search for rerank has no code results. Similarly for rename_field.

It does look like these used to be part of the smithy spec. but that code has been removed.

It looks like the request processors are included in https://github.com/opensearch-project/opensearch-api-specification/blob/main/spec/schemas/search_pipeline._common.yaml but I don't see any of the response types there

  • this may be a typo

Do you have any additional context?

The PRs for the two new processor types are targeted for 2.16.0 release.

I don't have the bandwidth to manually recreate all the response processors prior to 2.16.0 code freeze, and I'm hopeful automation already exists to translate them from the old Smithy specs.

@dbwiddis dbwiddis added bug Something isn't working untriaged labels Jul 18, 2024
@dblock dblock added enhancement New feature or request and removed bug Something isn't working untriaged labels Jul 18, 2024
@dblock dblock changed the title [BUG] Specs for Search Pipeline SearchResponseProcessors are missing Add Specs for Search Pipeline SearchResponseProcessors Jul 18, 2024
@nhtruong
Copy link
Collaborator

nhtruong commented Jul 18, 2024

What caused this

This was caused by a typo in the old search_pipeline.smithy file:

list ResponseProcessorList {
    member: RequestProcessor // should be ResponseProcessor
}

union ResponseProcessor was, therefore, never used anywhere. The built-in Smithy -> OpenAPI tool, as a result, also did not include it in the original OpenAPI file. This file was used to create the spec we see today. Hence, the typo was also carried over.

Note that this kind of errors will be caught by our new spec's linter that checks for unused schemas. 😃

How to fix it

  • Check out the last Smithy commit
  • Correct the typo in search_pipeline.smithy
  • Run ./gradlew build to generate a new OpenSearch.openapi.json. Check out this workflow for more details.
  • Compare the diff of the new OpenSearch.openapi.json with the old file.
  • Copy the diff into this spec (i.e. correcting the carried over typo)

@dbwiddis
Copy link
Member Author

Thanks for the instructions, @nhtruong!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants