Skip to content

Commit

Permalink
Fix but deprecate parameter aliases in body parameter
Browse files Browse the repository at this point in the history
  • Loading branch information
pquentin committed Feb 18, 2024
1 parent 7d4a34b commit 5141eea
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 4 deletions.
24 changes: 21 additions & 3 deletions elasticsearch/_sync/client/utils.py
Expand Up @@ -299,6 +299,7 @@ def _merge_kwargs_no_duplicates(kwargs: Dict[str, Any], values: Dict[str, Any])
def _merge_body_fields_no_duplicates(
body: _TYPE_BODY, kwargs: Dict[str, Any], body_fields: Tuple[str, ...]
) -> None:
mixed_body_and_params = False
for key in list(kwargs.keys()):
if key in body_fields:
if isinstance(body, (str, bytes)):
Expand All @@ -315,11 +316,13 @@ def _merge_body_fields_no_duplicates(
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.",
"version. Instead, use only 'body' or only specific parameters.",
category=DeprecationWarning,
stacklevel=warn_stacklevel(),
)
body[key] = kwargs.pop(key)
mixed_body_and_params = True
return mixed_body_and_params


def _rewrite_parameters(
Expand Down Expand Up @@ -401,6 +404,7 @@ def wrapped(*args: Any, **kwargs: Any) -> Any:
not ignore_deprecated_options or "body" not in ignore_deprecated_options
):
body: Optional[_TYPE_BODY] = kwargs.pop("body")
mixed_body_and_params = False
if body is not None:
if body_name:
if body_name in kwargs:
Expand All @@ -411,11 +415,25 @@ def wrapped(*args: Any, **kwargs: Any) -> Any:
"issues/1698 for more information"
)
kwargs[body_name] = body

elif body_fields is not None:
_merge_body_fields_no_duplicates(body, kwargs, body_fields)
mixed_body_and_params = _merge_body_fields_no_duplicates(
body, kwargs, body_fields
)
kwargs["body"] = body

if parameter_aliases and not isinstance(body, (str, bytes)):
for alias, rename_to in parameter_aliases.items():
if rename_to in body:
body[alias] = body.pop(rename_to)
# If body and params are mixed, the alias may come from a param, in which case the warning below will not make sense.
if not mixed_body_and_params:
warnings.warn(
f"Using '{rename_to}' alias in 'body' is deprecated and will be removed in a future version of elasticsearch-py. "
f"Use '{alias}' directly instead.",
category=DeprecationWarning,
stacklevel=2,
)

if parameter_aliases:
for alias, rename_to in parameter_aliases.items():
try:
Expand Down
37 changes: 36 additions & 1 deletion test_elasticsearch/test_client/test_rewrite_parameters.py
Expand Up @@ -191,7 +191,7 @@ def test_body_fields_merge(self):
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."
"version. Instead, use only 'body' or only specific parameters."
)

def test_body_fields_conflict(self):
Expand Down Expand Up @@ -238,6 +238,41 @@ def test_parameter_aliases(self):
self.wrapped_func_aliases(source=["key3"])
assert self.calls[-1] == ((), {"source": ["key3"]})

def test_parameter_aliases_body(self):
with pytest.warns(
DeprecationWarning,
match=(
"Using 'source' alias in 'body' is deprecated and will be removed in a future version of elasticsearch-py. "
"Use '_source' directly instead."
),
):
self.wrapped_func_aliases(body={"source": ["key4"]})

# using the correct name does not warn
with warnings.catch_warnings():
warnings.simplefilter("error")
self.wrapped_func_aliases(body={"_source": ["key4"]})

def test_parameter_aliases_body_param(self):
with pytest.warns(
DeprecationWarning,
match=(
"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 parameters."
),
):
self.wrapped_func_aliases(
source=["key4"], body={"query": {"match_all": {}}}
)

# using the correct name does not warn
with warnings.catch_warnings():
warnings.simplefilter("error")
self.wrapped_func_aliases(
body={"query": {"match_all": {}}, "_source": ["key4"]}
)

@pytest.mark.parametrize("client_cls", [Elasticsearch, AsyncElasticsearch])
def test_positional_argument_error(self, client_cls):
client = client_cls("https://localhost:9200")
Expand Down

0 comments on commit 5141eea

Please sign in to comment.