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

Support version field in SearchClient.post_search() #1079

Merged
merged 11 commits into from
Oct 16, 2024
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Added
~~~~~

- ``SearchQueryV1`` is a new class for submitting complex queries replacing
the legacy ``SearchQuery`` class. A deprecation warning has been added to the
``SearchQuery`` class. (:pr:`1079`)
4 changes: 4 additions & 0 deletions docs/services/search.rst
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ only to document the methods it provides to its subclasses.
:members:
:show-inheritance:

.. autoclass:: SearchQueryV1
m1yag1 marked this conversation as resolved.
Show resolved Hide resolved
:members:
:show-inheritance:

.. autoclass:: SearchScrollQuery
:members:
:show-inheritance:
Expand Down
3 changes: 3 additions & 0 deletions src/globus_sdk/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ def _force_eager_imports() -> None:
"SearchAPIError",
"SearchClient",
"SearchQuery",
"SearchQueryV1",
"SearchScrollQuery",
},
"services.timers": {
Expand Down Expand Up @@ -245,6 +246,7 @@ def _force_eager_imports() -> None:
from .services.search import SearchAPIError
from .services.search import SearchClient
from .services.search import SearchQuery
from .services.search import SearchQueryV1
from .services.search import SearchScrollQuery
from .services.timers import TimersAPIError
from .services.timers import TimersClient
Expand Down Expand Up @@ -386,6 +388,7 @@ def __getattr__(name: str) -> t.Any:
"SearchAPIError",
"SearchClient",
"SearchQuery",
"SearchQueryV1",
"SearchScrollQuery",
"SpecificFlowClient",
"StorageGatewayDocument",
Expand Down
2 changes: 2 additions & 0 deletions src/globus_sdk/_generate_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,9 @@ def __getattr__(name: str) -> t.Any:
(
"SearchAPIError",
"SearchClient",
# legacy class (remove in the future)
"SearchQuery",
"SearchQueryV1",
"SearchScrollQuery",
),
),
Expand Down
10 changes: 8 additions & 2 deletions src/globus_sdk/services/search/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
from .client import SearchClient
from .data import SearchQuery, SearchScrollQuery
from .data import SearchQuery, SearchQueryV1, SearchScrollQuery
from .errors import SearchAPIError

__all__ = ("SearchClient", "SearchQuery", "SearchScrollQuery", "SearchAPIError")
__all__ = (
"SearchClient",
"SearchQuery",
"SearchQueryV1",
"SearchScrollQuery",
"SearchAPIError",
)
54 changes: 53 additions & 1 deletion src/globus_sdk/services/search/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import typing as t

from globus_sdk import utils
from globus_sdk import exc, utils

# workaround for absence of Self type
# for the workaround and some background, see:
Expand Down Expand Up @@ -115,6 +115,7 @@ def __init__(
additional_fields: dict[str, t.Any] | None = None,
):
super().__init__()
exc.warn_deprecated("'SearchQuery' is deprecated. Use 'SearchQueryV1' instead.")
if q is not None:
self["q"] = q
if limit is not None:
Expand Down Expand Up @@ -221,6 +222,57 @@ def add_sort(
return self


class SearchQueryV1(utils.PayloadWrapper):
"""
A specialized dict which has helpers for creating and modifying a Search
Query document. Replaces the usage of ``SearchQuery``.

:param q: The query string. Required unless filters are used.
:param limit: A limit on the number of results returned in a single page
:param offset: An offset into the set of all results for the query
:param advanced: Whether to enable (``True``) or not to enable (``False``) advanced
parsing of query strings. The default of ``False`` is robust and guarantees that
the query will not error with "bad query string" errors
:param filters: a list of filters to apply to the query
:param facets: a list of facets to apply to the query
:param post_facet_filters: a list of filters to apply after facet
results are returned
:param boosts: a list of boosts to apply to the query
m1yag1 marked this conversation as resolved.
Show resolved Hide resolved
:param sort: a list of fields to sort results
:param additional_fields: additional data to include in the query document
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little surprised that pylint is passing, given that we have some undocumented parameters. Maybe I need to look at its settings for __init__... Regardless, we need param doc for filters, etc. It should be pretty short and easy to add.

Copy link
Member

Choose a reason for hiding this comment

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

pylint catches that? PyCharm notifies me about parameter mismatches in docstrings but I haven't spotted that in other linters.

Copy link
Member

Choose a reason for hiding this comment

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

Well, it's supposed to, under our config:

[tool.pylint]
load-plugins = ["pylint.extensions.docparams"]
accept-no-param-doc = "false"

[tool.pylint."messages control"]
disable = [  # simplified -- we have other disabled rules
    "missing-raises-doc",
]

But I'm guessing that it doesn't understand that the class docstring applies to the init method. Maybe it's configurable and we can fix it.

"""

def __init__(
self,
*,
q: str | utils.MissingType = utils.MISSING,
limit: int | utils.MissingType = utils.MISSING,
offset: int | utils.MissingType = utils.MISSING,
advanced: bool | utils.MissingType = utils.MISSING,
filters: list[dict[str, t.Any]] | utils.MissingType = utils.MISSING,
facets: list[dict[str, t.Any]] | utils.MissingType = utils.MISSING,
post_facet_filters: list[dict[str, t.Any]] | utils.MissingType = utils.MISSING,
boosts: list[dict[str, t.Any]] | utils.MissingType = utils.MISSING,
sort: list[dict[str, t.Any]] | utils.MissingType = utils.MISSING,
additional_fields: dict[str, t.Any] | utils.MissingType = utils.MISSING,
):
super().__init__()
self["@version"] = "query#1.0.0"

self["q"] = q
self["limit"] = limit
self["offset"] = offset
self["advanced"] = advanced
self["filters"] = filters
self["facets"] = facets
self["post_facet_filters"] = post_facet_filters
self["boosts"] = boosts
self["sort"] = sort

if not isinstance(additional_fields, utils.MissingType):
self.update(additional_fields)


class SearchScrollQuery(SearchQueryBase):
"""
A scrolling query type, for scrolling the full result set for an index.
Expand Down
73 changes: 61 additions & 12 deletions tests/functional/services/search/test_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,7 @@ def test_search_query_simple(search_client):
}


@pytest.mark.parametrize(
"query_doc",
[
{"q": "foo"},
{"q": "foo", "limit": 10},
globus_sdk.SearchQuery("foo"),
],
)
@pytest.mark.parametrize("query_doc", [{"q": "foo"}, {"q": "foo", "limit": 10}])
def test_search_post_query_simple(search_client, query_doc):
meta = load_response(search_client.post_search).metadata

Expand All @@ -63,12 +56,44 @@ def test_search_post_query_simple(search_client, query_doc):
assert req_body == dict(query_doc)


@pytest.mark.filterwarnings("ignore:'SearchQuery'*:DeprecationWarning")
def test_search_post_query_with_legacy_helper(search_client):
meta = load_response(search_client.post_search).metadata
query_doc = globus_sdk.SearchQuery("foo")

res = search_client.post_search(meta["index_id"], query_doc)
assert res.http_status == 200

data = res.data
assert isinstance(data, dict)
assert data["gmeta"][0]["entries"][0]["content"]["foo"] == "bar"

req = get_last_request()
assert req.body is not None
req_body = json.loads(req.body)
assert req_body == dict(query_doc)


def test_search_post_query_simple_with_v1_helper(search_client):
query_doc = globus_sdk.SearchQueryV1(q="foo")
meta = load_response(search_client.post_search).metadata

res = search_client.post_search(meta["index_id"], query_doc)
assert res.http_status == 200

data = res.data
assert isinstance(data, dict)
assert data["gmeta"][0]["entries"][0]["content"]["foo"] == "bar"

req = get_last_request()
assert req.body is not None
req_body = json.loads(req.body)
assert req_body == {"@version": "query#1.0.0", "q": "foo"}


@pytest.mark.parametrize(
"query_doc",
[
{"q": "foo", "limit": 10, "offset": 0},
globus_sdk.SearchQuery("foo", limit=10, offset=0),
],
[{"q": "foo", "limit": 10, "offset": 0}],
)
def test_search_post_query_arg_overrides(search_client, query_doc):
meta = load_response(search_client.post_search).metadata
Expand All @@ -92,6 +117,30 @@ def test_search_post_query_arg_overrides(search_client, query_doc):
assert query_doc["offset"] == 0


@pytest.mark.filterwarnings("ignore:'SearchQuery'*:DeprecationWarning")
def test_search_post_query_arg_overrides_with_legacy_helper(search_client):
meta = load_response(search_client.post_search).metadata
query_doc = globus_sdk.SearchQuery("foo", limit=10, offset=0)

res = search_client.post_search(meta["index_id"], query_doc, limit=100, offset=150)
assert res.http_status == 200

data = res.data
assert isinstance(data, dict)
assert data["gmeta"][0]["entries"][0]["content"]["foo"] == "bar"

req = get_last_request()
assert req.body is not None
req_body = json.loads(req.body)
assert req_body != dict(query_doc)
assert req_body["q"] == query_doc["q"]
assert req_body["limit"] == 100
assert req_body["offset"] == 150
# important! these should be unchanged (no side-effects)
assert query_doc["limit"] == 10
assert query_doc["offset"] == 0


@pytest.mark.parametrize(
"query_doc",
[
Expand Down
44 changes: 41 additions & 3 deletions tests/unit/helpers/test_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@

import pytest

from globus_sdk import SearchQuery
from globus_sdk import SearchQuery, SearchQueryV1, utils
from globus_sdk.exc.warnings import RemovedInV4Warning


def test_init():
@pytest.mark.filterwarnings("ignore:'SearchQuery'*:DeprecationWarning")
def test_init_legacy():
"""Creates SearchQuery and verifies results"""
# default init
query = SearchQuery()

assert len(query) == 0

# init with supported fields
Expand All @@ -26,7 +28,39 @@ def test_init():
assert param_query[par] == add_params[par]


def test_init_legacy_deprecation_warning():
with pytest.warns(
RemovedInV4Warning,
match="'SearchQuery' is deprecated. Use 'SearchQueryV1' instead.",
):
SearchQuery()


def test_init_v1():
query = SearchQueryV1()

# ensure the version is set to query#1.0.0
assert query["@version"] == "query#1.0.0"

# ensure key attributes initialize to empty lists
for attribute in ["facets", "filters", "post_facet_filters", "sort", "boosts"]:
assert query[attribute] == utils.MISSING

# init with supported fields
params = {"q": "foo", "limit": 10, "offset": 0, "advanced": False}
param_query = SearchQueryV1(**params)
for par in params:
assert param_query[par] == params[par]

# init with additional_fields
add_params = {"param1": "value1", "param2": "value2"}
param_query = SearchQueryV1(additional_fields=add_params)
for par in add_params:
assert param_query[par] == add_params[par]
m1yag1 marked this conversation as resolved.
Show resolved Hide resolved


@pytest.mark.parametrize("attrname", ["q", "limit", "offset", "advanced"])
@pytest.mark.filterwarnings("ignore:'SearchQuery'*:DeprecationWarning")
def test_set_method(attrname):
query = SearchQuery()
method = getattr(query, "set_{}".format("query" if attrname == "q" else attrname))
Expand All @@ -38,6 +72,7 @@ def test_set_method(attrname):
assert query[attrname] == "foo"


@pytest.mark.filterwarnings("ignore:'SearchQuery'*:DeprecationWarning")
def test_add_facet():
query = SearchQuery()
assert "facets" not in query
Expand Down Expand Up @@ -93,6 +128,7 @@ def test_add_facet():
}


@pytest.mark.filterwarnings("ignore:'SearchQuery'*:DeprecationWarning")
def test_add_filter():
query = SearchQuery()
assert "filters" not in query
Expand Down Expand Up @@ -133,6 +169,7 @@ def test_add_filter():
}


@pytest.mark.filterwarnings("ignore:'SearchQuery'*:DeprecationWarning")
def test_add_boost():
query = SearchQuery()
assert "boosts" not in query
Expand All @@ -154,6 +191,7 @@ def test_add_boost():
}


@pytest.mark.filterwarnings("ignore:'SearchQuery'*:DeprecationWarning")
def test_add_sort():
query = SearchQuery()
assert "sort" not in query
Expand Down