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

[BUG] v2.7.0 brings undocumented breaking change to put_index() #803

Closed
Tenzer opened this issue Aug 21, 2024 · 7 comments · Fixed by #804
Closed

[BUG] v2.7.0 brings undocumented breaking change to put_index() #803

Tenzer opened this issue Aug 21, 2024 · 7 comments · Fixed by #804
Labels
bug Something isn't working untriaged Need triage

Comments

@Tenzer
Copy link
Contributor

Tenzer commented Aug 21, 2024

What is the bug?

With versions prior to version 2.7.0 it was possible to call client.indices.put_alias("index_name", "alias_name") and have it create the alias.

With version 2.7.0 you instead get a cryptic error message along the lines of:

opensearchpy.exceptions.RequestError: RequestError(400, 'json_parse_exception', "Unrecognized token 'alias_name': was expecting (JSON String, Number, Array, Object or token 'null', 'true' or 'false')\n at [Source: REDACTED (StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION disabled); line: 1, column: 13]")

How can one reproduce the bug?

  1. Create an index
  2. Attempt to create an alias for it with client.indices.put_alias("index_name", "alias_name")

What is the expected behavior?

The alias should be created for the index like in previous opensearch-py versions.

What is your host/environment?

N/A

Do you have any screenshots?

N/A

Do you have any additional context?

This is caused by an inadvertently breaking change in #777 which changed the order of the arguments to the put_alias() method, meaning anybody who called the method without naming the arguments now supply their values to different arguments.

@Tenzer Tenzer added bug Something isn't working untriaged Need triage labels Aug 21, 2024
@Tenzer
Copy link
Contributor Author

Tenzer commented Aug 21, 2024

As mentioned in #777 (comment), this probably doesn't affect that many people, so perhaps the most pragmatic step is to just document this breaking change in the release notes for version 2.7.0.

@dblock
Copy link
Member

dblock commented Aug 21, 2024

@Tenzer we follow semver, so a breaking change should be fixed. Want to help? Otherwise I'll pick it up as soon as I can.

@Tenzer
Copy link
Contributor Author

Tenzer commented Aug 21, 2024

I guess it depends on what the optimal way to fix this would be. Would it require changes to the code generator or can it be changed directly in indices.py?

@dblock
Copy link
Member

dblock commented Aug 21, 2024

I guess it depends on what the optimal way to fix this would be. Would it require changes to the code generator or can it be changed directly in indices.py?

We need to write a test that reproduces the problem, then fix the generator, or the code will get overwritten. A hacky version is fine though.

@Tenzer
Copy link
Contributor Author

Tenzer commented Aug 21, 2024

I started looking into it. I've got a test written, and then looked into the code generation.

I guess we need to add some conditionals to the template for the function arguments: https://github.com/opensearch-project/opensearch-py/blob/main/utils/templates/func_params. It would need to check if it's outputting for the "put_alias" method, and then output the arguments in a different order.

@dblock
Copy link
Member

dblock commented Aug 21, 2024

I guess we need to add some conditionals to the template for the function arguments: https://github.com/opensearch-project/opensearch-py/blob/main/utils/templates/func_params. It would need to check if it's outputting for the "put_alias" method, and then output the arguments in a different order.

Yes, I would be perfectly fine with a very hacky solution, but I also have been thinking about something more structured like for example a folder with code patches (generated via git diff) that are applied after the code generator runs, to avoid the whole custom handling per method like here.

@Tenzer
Copy link
Contributor Author

Tenzer commented Aug 22, 2024

I noticed there was already code in place to override certain Jinja templates, so I made use of that instead. See #804.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working untriaged Need triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants