-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Fix environment variable casting for dict and list types #8494
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
Conversation
| + ' "mem_limit": "2g",' | ||
| + ' "cpu_count": 2,' | ||
| + ' "environment": {"TEST_VAR": "test_value"}' | ||
| + '}', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for fixing this! I'm not sure why does the agent assume this would be json, though?
The format is toml, these are attributes set in config.toml, and if the user wants, they can set and access them if set in env, instead.
|
@OpenHands read |
|
I'm on it! neubig can track my progress at all-hands.dev |
…l_eval instead of json.loads
|
I've addressed the comment on PR #8494 regarding the environment variable casting issue. Here's a summary of what I did: Changes Made
ConclusionThe changes successfully address @enyst's comment by:
The PR now uses the same approach for parsing complex types as the rest of the codebase, while still fixing the original issue of supporting both direct and origin-based type checking for dicts and lists. |
|
Thanks @enyst fixed! |
enyst
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
This PR fixes issue #8493 by improving the environment variable casting logic for dict and list types.
The issue was in the
utils.pyfile where the code was not properly handling directdictorlisttypes when casting environment variables. The fix adds support for bothfield_type is dict/listandget_origin(field_type) is dict/listcases, and usesjson.loadsfirst with a fallback toliteral_evalfor better compatibility.Added a test case to verify the fix works correctly with dictionary environment variables.
Closes #8493
@neubig can click here to continue refining the PR
To run this PR locally, use the following command: