Repository navigation
Python: fix OpenAPI plugin sending array and boolean parameters as Python repr - #14789
Jeevan Babu Gotru (jeevan1098) wants to merge 2 commits into
Conversation
GitHub Actions
Active
semantic-kernel-automation
Bot
added
the
python
There was a problem hiding this comment.
🟢 Approval recommended
There are no active review issues for the query-array and boolean serialization changes.
No open issues 🎉
What changed in this PR
Fixes #14788 by updating the Python OpenAPI plugin to serialize array query parameters and booleans in wire-compatible formats rather than Python representations.
Changes:
- Encodes query arrays as repeated keys using
doseq=True. - Serializes booleans as lowercase
true/falsein query, path, and header parameters. - Adds four regression tests. Parsing explicit OpenAPI
style/explodesettings remains out of scope.
| File | Description |
|---|---|
python/tests/unit/connectors/openapi_plugin/test_sk_openapi.py |
Adds regression coverage for query arrays and boolean query, path, and header values. |
python/semantic_kernel/connectors/openapi_plugin/models/rest_api_operation.py |
Adds boolean serialization and repeated-key query array encoding. |
Review effort: Balanced
Muhammad Abdullah Butt (1AbdullahButt)
left a comment
Muhammad Abdullah Butt (1AbdullahButt)
left a comment
There was a problem hiding this comment.
The query-string fix is right, but arrays in path and header parameters still go out as the Python list repr, which is the same bug in the other two functions this PR changes.
The issue: build_path and build_headers still str() a list. Both now call _serialize_parameter_value(argument) (rest_api_operation.py:229 and :343), but that helper only maps bool and returns everything else unchanged. An array header such as {"X-Tags": ["red", "blue"]} is still sent as ['red', 'blue']. An array path parameter becomes %5B%27red%27%2C%20%27blue%27%5D. A boolean inside a list isn't converted either ([True]). run_openapi_operation passes the kernel arguments through unchanged (openapi_manager.py:138-148), so a list from function calling reaches these functions as a list. The OpenAPI default for path and header parameters is style: simple, which is comma-separated (red,blue).
Suggested fix: handle sequences in the helper, so each call site doesn't need its own branch. For example, add a _serialize_simple that returns ",".join(str(self._serialize_parameter_value(v)) for v in value) for a list or tuple, and use it in build_path and build_headers. build_query_string keeps its own list branch for the form/explode case. A header test and a path test with ["red", "blue"] would cover it. If arrays in path and header are out of scope, a sentence in the description saying so would help, since the PR title and body talk about array parameters in general.
What I checked that is fine:
- The query path is the one the runner uses.
openapi_runner.py:123builds the URL withoperation.build_query_string(arguments), and nothing else encodes query parameters. doseq=Trueonly changes sequence values.urlencodetreatsstrandbytesas scalars even withdoseq=True, so string arguments encode as before. Ints go throughstr()as before. An empty list now leaves the key out instead of sending%5B%5D, which matchesform/explode.boolis checked before anything else, soTrue/Falsenever fall through tostr().isinstance(1, bool)is false, so integer0/1values are left alone.git merge-treeagainstmainis clean.
I didn't run the tests.
Reviewed with help from Claude Code.
Purpose
Fixes #14788
The Python OpenAPI plugin built query strings with
urlencode(segments)and passed argument values through unchanged. Withoutdoseq=True,urlencodecallsstr()on each value, so anarrayquery parameter was sent as the Python list repr (tags=%5B%27red%27%2C+%27blue%27%5D) and abooleanasactive=True— neither is parseable by the API. OpenAPI's default for query parameters isstyle: form, explode: true, i.e.?tags=red&tags=blue&active=true, which is also how the .NET plugin serializes arrays.Description
RestApiOperation.build_query_stringnow encodes withdoseq=True, so list/tuple values become repeated keys._serialize_parameter_valuehelper serializes booleans with the JSON spelling (true/false); it is applied to query values (including items inside arrays), and to path and header parameters, which had the samestr(argument)boolean problem.style/explodefrom the spec are still not parsed (as noted on the issue); this implements the OpenAPI default only.Reproduced with the issue's offline repro (httpx MockTransport) on main at 25bd1bb: before the fix the captured URL was
https://93.184.216.34/items?tags=%5B%27red%27%2C+%27blue%27%5D&active=True; after the fix it ishttps://93.184.216.34/items?tags=red&tags=blue&active=true.Regression tests
Four new tests in
test_sk_openapi.py: array + boolean query parameters,false+ boolean-array query parameters, a boolean path parameter, and a boolean header parameter. All four fail without the source fix. The mapped test file passes (83 tests); the fulltests/unit/connectors/openapi_plugin/suite shows only the pre-existing DNS-pinning failures, which are identical on clean main in this sandbox. Ruff check and format are clean on both changed files.This change was prepared with AI assistance; all steps above were run and verified by the contributor.