fix: convert Python booleans to valid Python literals in Kestra.outputs()#339
fix: convert Python booleans to valid Python literals in Kestra.outputs()#339nancysangani wants to merge 2 commits intokestra-io:mainfrom
Conversation
…uts() to prevent NameError in downstream tasks
There was a problem hiding this comment.
Pull request overview
This PR updates the Python helper library (kestra.py) and corresponding Java tests to treat boolean output values emitted via Kestra.outputs(...) as strings (e.g., "True") rather than JSON booleans.
Changes:
- Added a recursive boolean-to-string conversion in
Kestra.outputs(...)before JSON serialization. - Updated Python plugin tests to assert
"True"(string) instead oftrue(boolean) for output vars.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| plugin-script-python/src/main/resources/kestra.py | Adds boolean stringification logic for values passed to Kestra.outputs(...). |
| plugin-script-python/src/test/java/io/kestra/plugin/scripts/python/ScriptTest.java | Updates expected bool output var type/value in plugin-level script test. |
| plugin-script-python/src/test/java/io/kestra/core/tasks/scripts/PythonTest.java | Updates expected boolean output var type/value in deprecated core Python task tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| assertThat(run.getVars().get("test"), is("value")); | ||
| assertThat(run.getVars().get("int"), is(2)); | ||
| assertThat(run.getVars().get("bool"), is(true)); | ||
| assertThat(run.getVars().get("bool"), is("True")); |
| @staticmethod | ||
| def _convert_booleans(obj): | ||
| if isinstance(obj, bool): | ||
| return str(obj) | ||
| elif isinstance(obj, dict): | ||
| return {k: Kestra._convert_booleans(v) for k, v in obj.items()} | ||
| elif isinstance(obj, list): | ||
| return [Kestra._convert_booleans(item) for item in obj] | ||
| return obj | ||
|
|
||
| @staticmethod | ||
| def outputs(map): | ||
| Kestra._send({ | ||
| "outputs": map | ||
| "outputs": Kestra._convert_booleans(map) | ||
| }) |
|
|
||
| assertThat(run.getExitCode(), is(0)); | ||
| assertThat(run.getVars().get("ok"), is(true)); | ||
| assertThat(run.getVars().get("ok"), is("True")); |
| assertThat(run.getVars().get("bool"), is("True")); | ||
| assertThat(run.getVars().get("float"), is(3.65)); | ||
|
|
||
| assertThat(run.getVars().get("test"), is("value")); | ||
| assertThat(run.getVars().get("int"), is(2)); | ||
| assertThat(run.getVars().get("bool"), is(true)); | ||
| assertThat(run.getVars().get("bool"), is("True")); |
|
/cc @fdelbrayelle |
fdelbrayelle
added
area/plugin
fdelbrayelle
left a comment
fdelbrayelle
left a comment
There was a problem hiding this comment.
@nancysangani Proove it works by sharing screenshots of passing flows + logs please 🙏
|
@fdelbrayelle after looking into this further, it looks like the fix spans two repos. The kestraLibs test installs kestra directly from PyPI ( I’ve opened a corresponding PR there with the same fix — kestra-io/libs#39 Could you advise how you'd prefer to handle the kestraLibs test? Should it use the local bundled |
|
@fdelbrayelle Here are the screenshots proving the bug and the fix. Screenshot 1 (before fix): val = true causes NameError: name 'true' is not defined in the downstream task Screenshot 2 (after fix): Both tasks pass successfully once _convert_booleans() is applied. The |
Fixes #191
Problem
When
Kestra.outputs()is called with a Python boolean,json.dumpsserializes it as JSONtrue/false. When a downstream Python task references it via{{outputs.task.vars.my_bool}}, Pebble renders it astrue— not a valid Python identifier, causing aNameErrorin the downstream task.Fix
Added
_convert_booleans()inkestra.pythat recursively convertsboolvalues to"True"/"False"strings before serialization. All other types and languages are unaffected.Files Changed
kestra.py— added_convert_booleans(), updatedoutputs()to call itPythonTest.java— updated bool assertions to match new string representationScriptTest.java— same