-
Notifications
You must be signed in to change notification settings - Fork 121
[Python] Fix unicode support #2873
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
fa5859f to
3e597da
Compare
3e597da to
415631e
Compare
| resources: | ||
| jobs: | ||
| job_1: | ||
| name: "🔥🔥🔥" |
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.
nit: this tests unicode support in general, right? good to have, not specific to python, so I'd split this test.
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.
I think it isn't a problem for Golang code, because it's already using Unicode everywhere. It was just Python having a weird behavior
| _write_locations(f, locations) | ||
|
|
||
| with open(args.output, "w") as f: | ||
| with open(args.output, "w", encoding="utf-8") as f: |
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.
Rather than handling this case-by-case, should we set locale-related env vars so that everything in Python process (including libraries and subprocess) works with unicode?
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.
I think it can be weird if we start customizing the Python environment. For example, if customers write unit tests, they will have a different behaviour. We don't have any 3p dependencies and IO is very limited, so I think we should be just carefully specifying encoding where needed.
|
We can try again once #2870 is merged. |
## Changes Fix the Unicode support in the Python mutator. By default, `json.dump` escapes non-ASCII characters. We use the `ensure_ascii=False` to preserve the Unicode characters. ## Why Fixes #2859 ## Tests Acceptance and unit tests
## Release v0.252.0 ### Dependency updates * Upgraded Go SDK to 0.69.0 ([#2867](#2867)) * Upgraded to TF provider 1.79.0 ([#2869](#2869)) ### Bundles * Remove unused fields from resources.models schema: creation\_timestamp, last\_updated\_timestamp, latest\_versions and user\_id. Using them now raises a warning ([#2828](#2828)). * Preserve folder structure for app source code in bundle generate ([#2848](#2848)) * Fix normalising requirements file path in dependencies section ([#2861](#2861)) * Fix default-python template not to add environments when serverless=yes and include\_python=no ([#2866](#2866)) * Fix handling of Unicode characters in Python support ([#2873](#2873)) * Add support for secret scopes in DABs ([#2744](#2744)) * Make `artifacts.*.type` optional in bundle JSON schema ([#2881](#2881)) * Fix support for `spot_bid_max_price` field in Python support ([#2883](#2883))
Changes
Fix the Unicode support in the Python mutator.
By default,
json.dumpescapes non-ASCII characters. We use theensure_ascii=Falseto preserve the Unicode characters.Why
Fixes #2859
Tests
Acceptance and unit tests