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

Replace deprecated body in the ES requests #3043

Closed
obulat opened this issue Sep 19, 2023 · 8 comments · Fixed by #3055
Closed

Replace deprecated body in the ES requests #3043

obulat opened this issue Sep 19, 2023 · 8 comments · Fixed by #3055
Assignees
Labels
💻 aspect: code Concerns the software code in the repository 🛠 goal: fix Bug fix good first issue New-contributor friendly help wanted Open to participation from the community 🟧 priority: high Stalls work on the project or its dependents 🧱 stack: api Related to the Django API 🐍 tech: python Involves Python

Comments

@obulat
Copy link
Contributor

obulat commented Sep 19, 2023

Description

The get_sources function in the search_controller uses the deprecated body parameter:

if not sources:
# Don't increase `size` without reading this issue first:
# https://github.com/elastic/elasticsearch/issues/18838
size = 100
agg_body = {
"aggs": {
"unique_sources": {
"terms": {
"field": "source.keyword",
"size": size,
"order": {"_key": "desc"},
}
}
}
}
try:
results = settings.ES.search(index=index, body=agg_body, request_cache=True)

Deprecation warning in the CI (when running the API tests).

  /api/api/controllers/search_controller.py:682: DeprecationWarning: The 'body' parameter is deprecated and will be removed in a future version. Instead use individual parameters.
    results = settings.ES.search(index=index, body=agg_body, request_cache=True)

This can be replaced with the following code:

        aggs = {
                "unique_sources": {
                    "terms": {
                        "field": "source.keyword",
                        "size": size,
                        "order": {"_key": "desc"},
                    }
                }
        }
        try:
            results = settings.ES.search(index=index, aggs=aggs, request_cache=True)

Additional context

We recently updated ES version from 7 to 8, and started seeing the deprecation warnings in CI

@obulat obulat added good first issue New-contributor friendly help wanted Open to participation from the community 🟧 priority: high Stalls work on the project or its dependents 🛠 goal: fix Bug fix 💻 aspect: code Concerns the software code in the repository 🐍 tech: python Involves Python 🧱 stack: api Related to the Django API labels Sep 19, 2023
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Openverse Backlog Sep 19, 2023
@ngken0995
Copy link
Collaborator

Hello @obulat and @WordPress/openverse-maintainers,

I'm trying to have my first contribution on open source project and this issue look suitable for me. Can you assign me to this issue? How can I run an API tests?

@krysal
Copy link
Member

krysal commented Sep 19, 2023

Hi @ngken0995, you can find the instructions for the API tests in the docs site.

@ngken0995
Copy link
Collaborator

ngken0995 commented Sep 20, 2023

Hello @krysal,

Resolved: I have to download docker-compose.

Thank you for providing the docs site. It was super helpful. I can't join the slack channel because I'm not authorized. I'm trying to run just api/up.

Here is the result:
env COMPOSE_PROFILES="api" just ../up env COMPOSE_PROFILES="api" docker-compose -f docker-compose.yml up -d env: ‘docker-compose’: No such file or directory error: Recipe dcfailed on line 141 with exit code 127 env COMPOSE_PROFILES="api" docker-compose -f docker-compose.yml up -d env: ‘docker-compose’: No such file or directory error: Recipedcfailed on line 141 with exit code 127 env COMPOSE_PROFILES="api" docker-compose -f docker-compose.yml up -d env: ‘docker-compose’: No such file or directory error: Recipedcfailed on line 141 with exit code 127
I'm stuck trying to resolve the error. Please advise.

@krysal
Copy link
Member

krysal commented Sep 20, 2023

@ngken0995 Could try running just env before just api/up? That must complete your files.

Also, you should be able to join Slack, it's free to access for anyone, you just need a WordPress.org account first. Check this page.

@ngken0995
Copy link
Collaborator

Hello @krysal, I was able to run just api/up, just api/init and just api/test. I made the changes suggested above and still getting the same error when I run just api/ test. Do I need to change any test data?

@ngken0995
Copy link
Collaborator

ngken0995 commented Sep 21, 2023

@krysal, I have joined the slack channel with a WordPress.org account.

The error is the same from just api/test test/unit/controllers/test_search_controller.pywith the changes made. Does the package elasticsearch_dsl need to be updated? Here are the result below:

`test/unit/controllers/test_search_controller.py: 125 warnings
/venv/lib/python3.11/site-packages/elasticsearch_dsl/search.py:712: DeprecationWarning: Passing transport options in the API method is deprecated. Use 'Elasticsearch.options()' instead.
es.search(index=self._index, body=self.to_dict(), **self._params).body,

test/unit/controllers/test_search_controller.py: 237 warnings
/venv/lib/python3.11/site-packages/elasticsearch_dsl/search.py:712: DeprecationWarning: The 'body' parameter is deprecated and will be removed in a future version. Instead use individual parameters.
es.search(index=self._index, body=self.to_dict(), **self._params).body,

../venv/lib/python3.11/site-packages/_pytest/cacheprovider.py:451
/venv/lib/python3.11/site-packages/_pytest/cacheprovider.py:451: PytestCacheWarning: could not create cache path /api/.pytest_cache/v/cache/nodeids: [Errno 13] Permission denied: '/api/.pytest_cache'
config.cache.set("cache/nodeids", sorted(self.cached_nodeids))

../venv/lib/python3.11/site-packages/_pytest/stepwise.py:56
/venv/lib/python3.11/site-packages/_pytest/stepwise.py:56: PytestCacheWarning: could not create cache path /api/.pytest_cache/v/cache/stepwise: [Errno 13] Permission denied: '/api/.pytest_cache'
session.config.cache.set(STEPWISE_CACHE_DIR, [])

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

Results (86.18s (0:01:26)):
162 passed`

@obulat
Copy link
Contributor Author

obulat commented Sep 22, 2023

@ngken0995, It looks like all of the tests passed. There are some warnings about the deprecations, but they don't affect the 162 passed tests.

@ngken0995
Copy link
Collaborator

@obulat This is good to hear! I have created a PR.

@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Openverse Backlog Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💻 aspect: code Concerns the software code in the repository 🛠 goal: fix Bug fix good first issue New-contributor friendly help wanted Open to participation from the community 🟧 priority: high Stalls work on the project or its dependents 🧱 stack: api Related to the Django API 🐍 tech: python Involves Python
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants