Skip to content

Commit

Permalink
Accept body in _rewrite_parameters
Browse files Browse the repository at this point in the history
  • Loading branch information
pquentin committed Nov 29, 2023
1 parent f16d183 commit 7bae67e
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 25 deletions.
31 changes: 27 additions & 4 deletions docs/guide/migration.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ with the `security.grant_api_key` API:

[source,python]
------------------------------------
# 8.0+ SUPPORTED USAGES:
# 8.0+ SUPPORTED USAGE:
resp = (
client.options(
# This is the API key being used for the request
Expand All @@ -183,13 +183,36 @@ resp = (
)
)
# 7.x DEPRECATED USAGES (Don't do this!):
# 7.x DEPRECATED USAGE (Don't do this!):
resp = (
# This is the API key being used for the request
client.security.grant_api_key(
api_key=("request-id", "request-api-key"),
# This is the API key being granted
body={
# This is the API key being granted
"api_key": {
"name": "granted-api-key"
},
"grant_type": "password",
"username": "elastic",
"password": "changeme"
}
)
)
------------------------------------

Starting with the 8.12 client, using a body parameter is fully supported again, meaning you can also use `grant_api_key` like this:

[source,python]
------------------------------------
# 8.12+ SUPPORTED USAGE:
resp = (
client.options(
# This is the API key being used for the request
api_key=("request-id", "request-api-key")
).security.grant_api_key(
body={
# This is the API key being granted
"api_key": {
"name": "granted-api-key"
},
Expand Down Expand Up @@ -295,7 +318,7 @@ from elasticsearch import TransportError, Elasticsearch
try:
client.indices.get(index="index-that-does-not-exist")
# In elasticsearch-python v7.x this would capture the resulting
# In elasticsearch-py v7.x this would capture the resulting
# 'NotFoundError' that would be raised above. But in 8.0.0 this
# 'except TransportError' won't capture 'NotFoundError'.
except TransportError as err:
Expand Down
2 changes: 1 addition & 1 deletion elasticsearch/_async/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ def normalize_from_keyword(kw: MutableMapping[str, Any]) -> None:
search_kwargs = kwargs.copy()
search_kwargs["scroll"] = scroll
search_kwargs["size"] = size
resp = await client.search(body=query, **search_kwargs) # type: ignore[call-arg]
resp = await client.search(body=query, **search_kwargs)

scroll_id: Optional[str] = resp.get("_scroll_id")
scroll_transport_kwargs = pop_transport_kwargs(scroll_kwargs)
Expand Down
44 changes: 34 additions & 10 deletions elasticsearch/_sync/client/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@
str, Sequence[Union[str, Mapping[str, Union[str, int]], NodeConfig]]
]

_TYPE_BODY = Union[bytes, str, Dict[str, Any]]

_TYPE_ASYNC_SNIFF_CALLBACK = Callable[
[AsyncTransport, SniffOptions], Awaitable[List[NodeConfig]]
]
Expand Down Expand Up @@ -289,14 +291,40 @@ def _merge_kwargs_no_duplicates(kwargs: Dict[str, Any], values: Dict[str, Any])
if key in kwargs:
raise ValueError(
f"Received multiple values for '{key}', specify parameters "
"directly instead of using 'body' or 'params'"
"directly instead of using 'params'"
)
kwargs[key] = val


def _merge_body_fields_no_duplicates(
body: _TYPE_BODY, kwargs: Dict[str, Any], body_fields: Tuple[str, ...]
) -> None:
for key in list(kwargs.keys()):
if key in body_fields:
if isinstance(body, (str, bytes)):
raise ValueError(
"Couldn't merge 'body' with other parameters as it wasn't a mapping."
)

if key in body:
raise ValueError(
f"Received multiple values for '{key}', specify parameters "
"using either body or parameters, not both."
)

warnings.warn(
f"Received '{key}' via a specific parameter in the presence of a "
"'body' parameter, which is deprecated and will be removed in a future "
"version. Instead, use only 'body' or only specific paremeters.",
category=DeprecationWarning,
stacklevel=warn_stacklevel(),
)
body[key] = kwargs.pop(key)


def _rewrite_parameters(
body_name: Optional[str] = None,
body_fields: bool = False,
body_fields: Optional[Tuple[str, ...]] = None,
parameter_aliases: Optional[Dict[str, str]] = None,
ignore_deprecated_options: Optional[Set[str]] = None,
) -> Callable[[F], F]:
Expand Down Expand Up @@ -372,7 +400,7 @@ def wrapped(*args: Any, **kwargs: Any) -> Any:
if "body" in kwargs and (
not ignore_deprecated_options or "body" not in ignore_deprecated_options
):
body = kwargs.pop("body")
body: Optional[_TYPE_BODY] = kwargs.pop("body")
if body is not None:
if body_name:
if body_name in kwargs:
Expand All @@ -384,13 +412,9 @@ def wrapped(*args: Any, **kwargs: Any) -> Any:
)
kwargs[body_name] = body

elif body_fields:
if not hasattr(body, "items"):
raise ValueError(
"Couldn't merge 'body' with other parameters as it wasn't a mapping. "
"Instead of using 'body' use individual API parameters"
)
_merge_kwargs_no_duplicates(kwargs, body)
elif body_fields is not None:
_merge_body_fields_no_duplicates(body, kwargs, body_fields)
kwargs["body"] = body

if parameter_aliases:
for alias, rename_to in parameter_aliases.items():
Expand Down
2 changes: 1 addition & 1 deletion elasticsearch/helpers/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ def normalize_from_keyword(kw: MutableMapping[str, Any]) -> None:
search_kwargs = kwargs.copy()
search_kwargs["scroll"] = scroll
search_kwargs["size"] = size
resp = client.search(body=query, **search_kwargs) # type: ignore[call-arg]
resp = client.search(body=query, **search_kwargs)

scroll_id = resp.get("_scroll_id")
scroll_transport_kwargs = pop_transport_kwargs(scroll_kwargs)
Expand Down
52 changes: 43 additions & 9 deletions test_elasticsearch/test_client/test_rewrite_parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,19 @@ def wrapped_func_default(self, *args, **kwargs):
def wrapped_func_body_name(self, *args, **kwargs):
self.calls.append((args, kwargs))

@_rewrite_parameters(body_fields=True)
@_rewrite_parameters(body_fields=("query", "source"))
def wrapped_func_body_fields(self, *args, **kwargs):
self.calls.append((args, kwargs))

@_rewrite_parameters(
body_fields=True, ignore_deprecated_options={"api_key", "body", "params"}
body_fields=("query",), ignore_deprecated_options={"api_key", "body", "params"}
)
def wrapped_func_ignore(self, *args, **kwargs):
self.calls.append((args, kwargs))

@_rewrite_parameters(body_fields=True, parameter_aliases={"_source": "source"})
@_rewrite_parameters(
body_fields=("source",), parameter_aliases={"_source": "source"}
)
def wrapped_func_aliases(self, *args, **kwargs):
self.calls.append((args, kwargs))

Expand Down Expand Up @@ -81,6 +83,16 @@ def test_default(self):
((), {"query": {"match_all": {}}, "key": "value"}),
]

def test_default_params_conflict(self):
with pytest.raises(ValueError) as e:
self.wrapped_func_default(
query={"match_all": {}},
params={"query": {"match_all": {}}},
)
assert str(e.value) == (
"Received multiple values for 'query', specify parameters directly instead of using 'params'"
)

def test_body_name_using_body(self):
with warnings.catch_warnings(record=True) as w:
self.wrapped_func_body_name(
Expand Down Expand Up @@ -142,18 +154,21 @@ def test_body_fields(self):

assert self.calls == [
((), {"api_key": ("id", "api_key")}),
((), {"query": {"match_all": {}}}),
((), {"body": {"query": {"match_all": {}}}}),
]

@pytest.mark.parametrize(
"body", ['{"query": {"match_all": {}}}', b'{"query": {"match_all": {}}}']
"body, kwargs",
[
('{"query": {"match_all": {}}}', {"query": {"match_all": {}}}),
(b'{"query": {"match_all": {}}}', {"query": {"match_all": {}}}),
],
)
def test_error_on_body_merge(self, body):
def test_error_on_body_merge(self, body, kwargs):
with pytest.raises(ValueError) as e:
self.wrapped_func_body_fields(body=body)
self.wrapped_func_body_fields(body=body, **kwargs)
assert str(e.value) == (
"Couldn't merge 'body' with other parameters as it wasn't a mapping. Instead of "
"using 'body' use individual API parameters"
"Couldn't merge 'body' with other parameters as it wasn't a mapping."
)

@pytest.mark.parametrize(
Expand All @@ -167,6 +182,25 @@ def test_error_on_params_merge(self, params):
"using 'params' use individual API parameters"
)

def test_body_fields_merge(self):
with warnings.catch_warnings(record=True) as w:
self.wrapped_func_body_fields(source=False, body={"query": {}})

assert len(w) == 1
assert w[0].category == DeprecationWarning
assert str(w[0].message) == (
"Received 'source' via a specific parameter in the presence of a "
"'body' parameter, which is deprecated and will be removed in a future "
"version. Instead, use only 'body' or only specific paremeters."
)

def test_body_fields_conflict(self):
with pytest.raises(ValueError) as e:
self.wrapped_func_body_fields(query={"match_all": {}}, body={"query": {}})
assert str(e.value) == (
"Received multiple values for 'query', specify parameters using either body or parameters, not both."
)

def test_ignore_deprecated_options(self):
with warnings.catch_warnings(record=True) as w:
self.wrapped_func_ignore(
Expand Down

0 comments on commit 7bae67e

Please sign in to comment.