[BUG] passing timeout parameter to some functions in the OpenSearch object raises an exception.

[firas-rabaia]

What is the bug?

_all int, float parameters that are being passed using the "@query_params" decorator are being converted to a string, which sometimes raises an exception, happened to me on calling opensearchpy.client.OpenSearch.delete_by_query caused ValueError exception. _

How can one reproduce the bug?

_
1- find a function that uses the "@query_params" decorator
2- pass an int or float parameter
3- check the value of the passed parameter in the function to find it is not passed as it is called
_

What is the expected behavior?

values passed int or float should stay the same and not be converted/casted into strings.

What is your host/environment?

MAC OS 14.1, python 3.9, opensearch-py 2.2.0

Do you have any screenshots?

Do you have any additional context?

calling opensearchpy.client.OpenSearch.delete_by_query with params dictionary and adding any needed parameters works fine can be used as a solution for current and old versions.

[dblock]

Thanks for reporting this.

Appreciate it if you could help check that the parameters are properly described in https://github.com/opensearch-project/opensearch-api-specification and fix this issue here (possibly in the code generator).

[silkspace]

Seems related to my issue here

[firas-rabaia]

@dblock Will check it out, thx.
@silkspace yep, it is the same issue caused by the same helper/util function that is used almost everywhere, I didn't know there was a code generator wanna check it out first.

[firas-rabaia]

@dblock
found conflicting types between the API specs you sent me to check out and the code in for the timeout types for timeout param (int, float, None).
where the API with open API shows
opensearch-api timeout param

and the Duration schema is a string with a regex pattern timeout schema type (Duration)

while in code in the last PR as you mentioned it is int,float. PR where timeout introduced

I checked the urllib3 call where the exception Originate and it is int, float. urllib3 call

so we will need 2 fixes one for the API specs for the code generator for the timeout type to have a separate schema int,float and one for the util helper function that I already fixed(since I think it is not something generated by the code generation), am I right or missing something? thx for your comments and help ^^

[dblock]

One thing that's not clear to me is what does OpenSearch accept? The doc says "A duration. Units can be nanos, micros, ms (milliseconds), s (seconds), m (minutes), h (hours) and d (days). Also accepts "0" without a unit and "-1" to indicate an unspecified value." Is that correct? In which case it does not accept int, only a value of 0 or -1?

Try it out, let's know 100% for sure what it is? Then the client needs to match exactly that via code generator.

Finally, I do think one should be able to specify timeout = int in the client. That's a nice feature to have where we assume it's seconds and convert it to "10s" instead of "10" as a fix to this problem?

[silkspace]

the decorator is nice syntactic sugar, but nothing beats Typing.

[firas-rabaia]

POST index/_search
{
    "query": {
        "match": {
            "field": 1
        }
    },
    "timeout": "10s"
}

I have an instance I ran this in the dev-tools of the OpenSearch Instance UI.

"10s", 0, -1 works perfectly fine but try running it without timeout units with values like this 100 or "100", or as you said any other int value other than 0 or -1 it won't work.

{
  "error": {
    "root_cause": [
      {
        "type": "illegal_argument_exception",
        "reason": "failed to parse setting [timeout] with value [100] as a time value: unit is missing or unrecognized"
      }
    ],
    "type": "illegal_argument_exception",
    "reason": "failed to parse setting [timeout] with value [100] as a time value: unit is missing or unrecognized"
  },
  "status": 400
}

since this is consistent with what we assume we can consider that the timeout in the client should at least accept the same as specs but also add the integer feature if possible ? ofc in the code gen library.

just to re-iterate that we have a mutual agreement the issue is that the OpenSearch-py passes the timeout as a string since the util function converts int, and float types to a String, but the urilib3 lib and other libs used where we pass the timeout to it expects the timeout value to be an integer, we agree on that right?

much thx for your patience.

[dblock]

since this is consistent with what we assume we can consider that the timeout in the client should at least accept the same as specs but also add the integer feature if possible ? ofc in the code gen library.

Yes.

just to re-iterate that we have a mutual agreement the issue is that the OpenSearch-py passes the timeout as a string since the util function converts int, and float types to a String, but the urilib3 lib and other libs used where we pass the timeout to it expects the timeout value to be an integer, we agree on that right?

Yes. I think we need two functions, one that converts a timeout into what OpenSearch takes from any format, and another into seconds that's taken by urllib3 from any format. Plus tests for every possible combination at usage level.

[firas-rabaia]

@dblock
and this code is not something generated from the code gen, will add it to this repo only? but we may need to run code gen to verify nothing is non-compliant with the original spec, right ?
thx for your patience