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 but deprecate parameter aliases in body parameter #2427

Merged
merged 1 commit into from Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 24 additions & 4 deletions elasticsearch/_sync/client/utils.py
Expand Up @@ -298,7 +298,8 @@ 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:
) -> bool:
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,27 @@ 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 "
f"in a future version of elasticsearch-py. Use '{alias}' directly instead. "
"See https://github.com/elastic/elasticsearch-py/issues/1698 for more information",
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