-
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| bundle: | ||
| name: my_project | ||
|
|
||
| sync: { paths: [] } # don't need to copy files | ||
|
|
||
| experimental: | ||
| python: | ||
| resources: | ||
| - "resources:load_resources" | ||
|
|
||
| resources: | ||
| jobs: | ||
| job_1: | ||
| name: "🔥🔥🔥" | ||
|
|
||
| variables: | ||
| my_variable: | ||
| default: "my_variable" | ||
| description: "🔥🔥🔥" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
|
|
||
| >>> uv run --with-requirements requirements-latest.txt --no-cache -q [CLI] bundle validate --output json | ||
| Warning: This is a warning message with unicode characters: 🔥🔥🔥 | ||
|
|
||
| { | ||
| "variables": { | ||
| "my_variable": { | ||
| "default": "my_variable", | ||
| "description": "🔥🔥🔥", | ||
| "value": "my_variable" | ||
| } | ||
| }, | ||
| "resources": { | ||
| "jobs": { | ||
| "job_1": { | ||
| "deployment": { | ||
| "kind": "BUNDLE", | ||
| "metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/my_project/default/state/metadata.json" | ||
| }, | ||
| "edit_mode": "UI_LOCKED", | ||
| "format": "MULTI_TASK", | ||
| "max_concurrent_runs": 1, | ||
| "name": "🔥🔥🔥", | ||
| "permissions": [], | ||
| "queue": { | ||
| "enabled": true | ||
| } | ||
| }, | ||
| "job_2": { | ||
| "deployment": { | ||
| "kind": "BUNDLE", | ||
| "metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/my_project/default/state/metadata.json" | ||
| }, | ||
| "edit_mode": "UI_LOCKED", | ||
| "format": "MULTI_TASK", | ||
| "max_concurrent_runs": 1, | ||
| "name": "🔥🔥🔥", | ||
| "permissions": [], | ||
| "queue": { | ||
| "enabled": true | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| from databricks.bundles.core import Resources, Diagnostics | ||
|
|
||
|
|
||
| def load_resources() -> Resources: | ||
| resources = Resources() | ||
|
|
||
| resources.add_job("job_2", {"name": "🔥🔥🔥"}) | ||
|
|
||
| resources.add_diagnostics( | ||
| Diagnostics.create_warning("This is a warning message with unicode characters: 🔥🔥🔥"), | ||
| ) | ||
|
|
||
| return resources |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| echo "$DATABRICKS_BUNDLES_WHEEL" > "requirements-latest.txt" | ||
|
|
||
| trace uv run $UV_ARGS -q $CLI bundle validate --output json | \ | ||
| jq "pick(.variables, .resources)" | ||
|
|
||
| rm -fr .databricks __pycache__ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| Local = true | ||
| Cloud = false # tests don't interact with APIs | ||
|
|
||
| [EnvMatrix] | ||
| UV_ARGS = [ | ||
| # only fixed in the latest version | ||
| "--with-requirements requirements-latest.txt --no-cache", | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -169,7 +169,7 @@ def _apply_mutators_for_type( | |
| def python_mutator( | ||
| args: _Args, | ||
| ) -> tuple[dict, dict[tuple[str, ...], Location], Diagnostics]: | ||
| input = json.load(open(args.input)) | ||
| input = json.load(open(args.input, encoding="utf-8")) | ||
| experimental = input.get("experimental", {}) | ||
|
|
||
| if experimental.get("pydabs", {}) != {}: | ||
|
|
@@ -446,14 +446,14 @@ def main(argv: list[str]) -> int: | |
| logging.basicConfig(level=logging.DEBUG, stream=sys.stderr) | ||
| new_bundle, locations, diagnostics = python_mutator(args) | ||
|
|
||
| with open(args.diagnostics, "w") as f: | ||
| with open(args.diagnostics, "w", encoding="utf-8") as f: | ||
| _write_diagnostics(f, diagnostics) | ||
|
|
||
| if locations_path := args.locations: | ||
| with open(locations_path, "w") as f: | ||
| with open(locations_path, "w", encoding="utf-8") as f: | ||
| _write_locations(f, locations) | ||
|
|
||
| with open(args.output, "w") as f: | ||
| with open(args.output, "w", encoding="utf-8") as f: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| _write_output(f, new_bundle) | ||
|
|
||
| return 1 if diagnostics.has_error() else 0 | ||
|
|
@@ -466,12 +466,12 @@ def _write_diagnostics(f: TextIO, diagnostics: Diagnostics) -> None: | |
| if obj.get("path"): | ||
| obj["path"] = ".".join(obj["path"]) | ||
|
|
||
| json.dump(obj, f) | ||
| json.dump(obj, f, ensure_ascii=False) | ||
| f.write("\n") | ||
|
|
||
|
|
||
| def _write_output(f: TextIO, bundle: dict) -> None: | ||
| json.dump(bundle, f) | ||
| json.dump(bundle, f, ensure_ascii=False) | ||
|
|
||
|
|
||
| def _relativize_locations( | ||
|
|
@@ -503,7 +503,7 @@ def _write_locations(f: TextIO, locations: dict[tuple[str, ...], Location]) -> N | |
| for path, location in locations.items(): | ||
| obj = {"path": ".".join(path), **location.as_dict()} | ||
|
|
||
| json.dump(obj, f) | ||
| json.dump(obj, f, ensure_ascii=False) | ||
| f.write("\n") | ||
|
|
||
|
|
||
|
|
||
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