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

Fix AggsProxy can't be imported | functional.py | utils.py (Tested with elasticsearch-dsl==8.13.1 & django==5.0.4) #316

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
>>>
>>> model = Publisher # The model associate with this Document
"""
from elasticsearch_dsl.search import AggsProxy

Choose a reason for hiding this comment

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

We (elasticsearch-dsl team @ Elastic) cannot guarantee that this will continue to work in future releases of elasticsearch-dsl. AggsProxy is an internal class of this package, not intended to be used externally. Also consider that with this change users will be required to upgrade to elasticsearch-dsl==8.13.1 as any older versions will not work anymore.

In 8.13.1 we have added an implementation of the EmptySearch class that we intend to maintain going forward. My suggestion for a fix is something like this:

try:
    # code for 8.13 (requires 8.13.1)
    from elasticsearch import EmptySearch
except ImportError:
    # 8.12 and older
    # your definition of Empty Search here, including the original import of AggsProxy

Copy link

@millerf millerf Nov 24, 2024

Choose a reason for hiding this comment

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

@miguelgrinberg I started implementing a solution in #319 .

However AggsProxy is used as well in another class. I am not sure how to tackle this one...

Choose a reason for hiding this comment

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

@millerf I have missed that second usage. Unfortunately that one is more difficult, because it uses a few other private items in addtion to the AggsProxy instance. The whole idea of updating a Search instance in place goes against our design.

The two ways I can think of to "clean" one of these objects are:

  • Create a brand new instance of the same class
  • Run a query that is guaranteed to return no results

Copy link

Choose a reason for hiding this comment

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

@millerf I have missed that second usage. Unfortunately that one is more difficult, because it uses a few other private items in addtion to the AggsProxy instance. The whole idea of updating a Search instance in place goes against our design.

The two ways I can think of to "clean" one of these objects are:

  • Create a brand new instance of the same class
  • Run a query that is guaranteed to return no results

Unfortunately I don't know enough about this plugin to get to do that... I am hoping someone will be able to help...

from elasticsearch_dsl.search_base import AggsProxy

from django_elasticsearch_dsl_drf.constants import (
FUNCTIONAL_SUGGESTER_TERM_MATCH,
Expand Down
2 changes: 1 addition & 1 deletion src/django_elasticsearch_dsl_drf/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"""

import datetime
from elasticsearch_dsl.search import AggsProxy
from elasticsearch_dsl.search_base import AggsProxy


__title__ = 'django_elasticsearch_dsl_drf.utils'
Expand Down