Added Tests for Search Pipeline and Notifications Plugin

[AbitraryYu]

Description

Describe what this change achieves.

This change tries to solve #474

Issues Resolved

List any issues this PR will resolve, e.g. Closes [...].

If this is resolved, this closes #474

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.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 56.52174% with 40 lines in your changes are missing coverage. Please review.

Project coverage is 71.80%. Comparing base (ba715b9) to head (3d7586c). Report is 11 commits behind head on main.

:exclamation: Current head 3d7586c differs from pull request most recent head 891c250. Consider uploading reports for the commit 891c250 to get more accurate results

Files	Patch %	Lines
opensearchpy/_async/plugins/notifications.py	53.57%	13 Missing :warning:
opensearchpy/plugins/notifications.py	53.57%	13 Missing :warning:
opensearchpy/_async/client/search_pipeline.py	50.00%	7 Missing :warning:
opensearchpy/client/search_pipeline.py	50.00%	7 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #668 +/- ## ========================================== - Coverage 71.95% 71.80% -0.16% ========================================== Files 91 95 +4 Lines 8001 8103 +102 ========================================== + Hits 5757 5818 +61 - Misses 2244 2285 +41 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[orangewise]

Question, would this also work with opensearch serverless (aws)?

[AbitraryYu]

I see there's a PR in opensearch-api-specification . If that PR is merged, then does that mean the search pipeline api for opensearch-py can be generated? Thanks.

[saimedhi]

I see there's a PR in opensearch-api-specification . If that PR is merged, then does that mean the search pipeline api for opensearch-py can be generated? Thanks.

• @AbitraryYu, once the spec is merged, we can generate the search pipeline API. After generation, we'll review it for any necessary fixes to the generator code.

• Your contribution to getting the spec merged would be greatly appreciated, and it will help us move forward with API generation. Thank you.

[lambda-science]

Any follow up on this ? .. We need search pipeline API

[saimedhi]

Any follow up on this ? .. We need search pipeline API

@lambda-science, @ArbitraryYu, please feel free to contribute to the spec to get it merged.

https://github.com/opensearch-project/opensearch-api-specification/pull/172

[AbitraryYu]

When I try to generate the code using nox -rs generate it fails at /utils/generate_api.py line 595. I have printed some of the debug logs (by inserting some print statements on my own) and saw the following:

{'name': 'config_id', 'in': 'query', 'description': 'A channel ID.', 'schema': {'type': 'string', 'description': 'A channel ID.'}}
{'name': 'config_id_list', 'in': 'query', 'description': 'A comma-separated list of channel IDs.', 'schema': {'type': 'string', 'description': 'A comma-separated list of channel IDs.'}}
{'name': 'last_updated_time_ms', 'in': 'query', 'schema': {'type': 'integer', 'format': 'int64'}}
Traceback (most recent call last):
  File "<repo_dir>/utils/generate_api.py", line 853, in <module>
    dump_modules(read_modules())
                 ^^^^^^^^^^^^^^
  File "<repo_dir>/utils/generate_api.py", line 595, in read_modules
    type=m["schema"]["type"], description=m["description"]
                                          ~^^^^^^^^^^^^^^^
KeyError: 'description'
nox > Command python utils/generate_api.py failed with exit code 1
nox > Session generate failed.

I take a look at the OpenSearch.openapi.json at raw.github, and find out it is the GET parameters of /_plugins/_notifications/configs that lack the description (on line 13377, or see the picture)

I saw some other parameters also lack description, I am not sure if there are more parameters that lack description.

My question is: Are there any methods that I can skip the problematic part and just generate the search pipeline API part?If not, is it a new issue that need to be raised in the opensearch-api-specification repository?

[saimedhi]

When I try to generate the code using nox -rs generate it fails at /utils/generate_api.py line 595. I have printed some of the debug logs (by inserting some print statements on my own) and saw the following:

{'name': 'config_id', 'in': 'query', 'description': 'A channel ID.', 'schema': {'type': 'string', 'description': 'A channel ID.'}}
{'name': 'config_id_list', 'in': 'query', 'description': 'A comma-separated list of channel IDs.', 'schema': {'type': 'string', 'description': 'A comma-separated list of channel IDs.'}}
{'name': 'last_updated_time_ms', 'in': 'query', 'schema': {'type': 'integer', 'format': 'int64'}}
Traceback (most recent call last):
  File "<repo_dir>/utils/generate_api.py", line 853, in <module>
    dump_modules(read_modules())
                 ^^^^^^^^^^^^^^
  File "<repo_dir>/utils/generate_api.py", line 595, in read_modules
    type=m["schema"]["type"], description=m["description"]
                                          ~^^^^^^^^^^^^^^^
KeyError: 'description'
nox > Command python utils/generate_api.py failed with exit code 1
nox > Session generate failed.

I take a look at the OpenSearch.openapi.json at raw.github, and find out it is the GET parameters of /_plugins/_notifications/configs that lack the description (on line 13377, or see the picture)

I saw some other parameters also lack description, I am not sure if there are more parameters that lack description.

My question is: Are there any methods that I can skip the problematic part and just generate the search pipeline API part?If not, is it a new issue that need to be raised in the opensearch-api-specification repository?

When I try to generate the code using nox -rs generate it fails at /utils/generate_api.py line 595. I have printed some of the debug logs (by inserting some print statements on my own) and saw the following:

{'name': 'config_id', 'in': 'query', 'description': 'A channel ID.', 'schema': {'type': 'string', 'description': 'A channel ID.'}}
{'name': 'config_id_list', 'in': 'query', 'description': 'A comma-separated list of channel IDs.', 'schema': {'type': 'string', 'description': 'A comma-separated list of channel IDs.'}}
{'name': 'last_updated_time_ms', 'in': 'query', 'schema': {'type': 'integer', 'format': 'int64'}}
Traceback (most recent call last):
  File "<repo_dir>/utils/generate_api.py", line 853, in <module>
    dump_modules(read_modules())
                 ^^^^^^^^^^^^^^
  File "<repo_dir>/utils/generate_api.py", line 595, in read_modules
    type=m["schema"]["type"], description=m["description"]
                                          ~^^^^^^^^^^^^^^^
KeyError: 'description'
nox > Command python utils/generate_api.py failed with exit code 1
nox > Session generate failed.

I take a look at the OpenSearch.openapi.json at raw.github, and find out it is the GET parameters of /_plugins/_notifications/configs that lack the description (on line 13377, or see the picture)

I saw some other parameters also lack description, I am not sure if there are more parameters that lack description.

My question is: Are there any methods that I can skip the problematic part and just generate the search pipeline API part?If not, is it a new issue that need to be raised in the opensearch-api-specification repository?

Hey @AbitraryYu, to avoid the generator error, we could tweak the code to first check if the parameter description exists before using it. Yet, I believe it's even better to include the parameter description that is missing in the spec.

@dblock, what's your take on this?

[saimedhi]

@AbitraryYu, please try fixing https://github.com/opensearch-project/opensearch-py/issues/710.

So, that all APIs from spec will be generated by update_api workflow and PR will be raised automatically if changes detected. Consider raising PR for spec repo with missing descriptions or fix generator to skip using description if not present. Thank you.

[dblock]

@dblock, what's your take on this?

If the description is not required, the generator should gracefully handle that.

[AbitraryYu]

The code is generated via nox, and I modified the generator in generate_api.py to gracefully ignore messages if the API specification is empty.

By the way, the raw link of the opensearch "https://raw.githubusercontent.com/opensearch-project/opensearch-api-specification/main/OpenSearch.openapi.json" seems giving me 404 (line 550-554), so I use my local version downloaded from last week.

[AbitraryYu]

A side note, the latest version of opensearch (docker) require admin password, and I cannot get it run tests locally. I am not sure if this repository supports password parsing so I am using an older version: 2.11.1 (docker).

[saimedhi]

@AbitraryYu, Thank you for implementing the requested changes! Could you also include tests for these generated APIs?

[AbitraryYu]

Hi @saimedhi , I got confused where I should implement the tests. I saw test_opensearch/test_helpers/test_search.py and test_opensearch/test_async/test_helpers/test_search.py, and they are using from opensearchpy.helpers import search. I am wondering do I need to create a new helper file in opensearchpy.helpers plus creating both new test files in test_opensearch/test_helpers/ and test_opensearch//test_async/test_helpers/. Thanks.

[saimedhi]

Hi @saimedhi , I got confused where I should implement the tests. I saw test_opensearch/test_helpers/test_search.py and test_opensearch/test_async/test_helpers/test_search.py, and they are using from opensearchpy.helpers import search. I am wondering do I need to create a new helper file in opensearchpy.helpers plus creating both new test files in test_opensearch/test_helpers/ and test_opensearch//test_async/test_helpers/. Thanks.

@AbitraryYu,

• For search_pipeline unit tests, they can be added here. Please create a file for search_pipeline test cases in that folder.
• For search_pipeline integration tests, removing the skip test here is sufficient.
• Notification plugin tests can be added here, and async tests here.

[saimedhi]

Thank you for your contribution, @ArbitraryYu. The Search Pipeline and Notification APIs have now been added to opensearch-py. We would greatly appreciate your help in adding tests for the Notifications Plugin whenever you have the chance.

[AbitraryYu]

Hi @saimedhi , I just added the tests by following this notification api link and this search pipeline link

[AbitraryYu]

Sorry for all the dirty commits, I accidentally committed some sensitive data to this branch and I attempt to remove it both locally and remotely. I already force push the git history so the my local repo feature branch cannot access it. However, this PR still have the stray commit that contain the sensitive data.

I just submitted a Github support request for to clear the cached views but might need this PR's admin permission to actually take effect.

Update: The affected commit seems to be hidden after 2 hours, would let you guys know if Github support need your support. Thank you.

Update 2: The GitHub support team removed the affected commit cache and I confirmed the affected commit is no longer accessible, so I think the case is closed. Thank you.

[saimedhi]

@ArbitraryYu, please resolve the failing DCO check.

[AbitraryYu]

For the DCO, I try to follow the DCO guide, git rebase HEAD~10 --signoff, going through the rebase process, and once I push it, the PR got 49+ files changed. I was wondering if I can add those signoff(s) by modifying a specific commit?

[dblock]

For the DCO, I try to follow the DCO guide, git rebase HEAD~10 --signoff, going through the rebase process, and once I push it, the PR got 49+ files changed. I was wondering if I can add those signoff(s) by modifying a specific commit?

It's probably easier to re-create the changes in the branch, or do https://code.dblock.org/2015/08/31/getting-out-of-your-first-git-mess.html.

git checkout feature/search_pipeline_api
git checkout -b feature/search_pipeline_api-backup
git checkout feature/search_pipeline_api
git reset HEAD~1 # repeat this until you've rewound your branch enough
git add .
git commit -s -m ...
git push origin feature/search_pipeline_api-backup -f

[AbitraryYu]

Fixed the DCO, and the tests seems ok.

[saimedhi]

@AbitraryYu, please try to fix failing tests.

[AbitraryYu]

I followed the example on test_index_management.py of this reference, and I got errors like:

• error: Unsupported target for indexed assignment ("Collection[str]") on here
• error: Value of type "Collection[str]" is not indexable on here.

How can I fix that?

Meanwhile, the test_list_all_config() returns an empty list of config list, so I didn't write an assertEqual test; while test_get_config() returns something on the config list. I am not sure if test_list_all_config() is behaving properly, both tests create an index self.test_create_config() at the start.

[saimedhi]

I followed the example on test_index_management.py of this reference, and I got errors like:

• error: Unsupported target for indexed assignment ("Collection[str]") on here
• error: Value of type "Collection[str]" is not indexable on here.

How can I fix that?

@ArbitraryYu, follow these steps to resolve the errors:

• Format the code using nox -rs format.
• Add type hints for CONTENT here:

CONTENT: Dict[str, Any] = {
    "config_id": "sample-id",
    # ... other keys ...
}

[saimedhi]

@dblock, I'm unable to merge the PR. Could you please review, approve, and merge it? Thank you!