moomindani commented on code in PR #70385:
URL: https://github.com/apache/airflow/pull/70385#discussion_r3670375957
##########
providers/databricks/src/airflow/providers/databricks/utils/databricks.py:
##########
@@ -17,9 +17,22 @@
# under the License.
from __future__ import annotations
+import re
+
from airflow.providers.common.compat.sdk import AirflowException, XComArg
from airflow.providers.databricks.hooks.databricks import DatabricksHook,
RunState
+_JSONB_INVALID_CHARS = re.compile(r"[\x00\ud800-\udfff]")
+
+
+def make_jsonb_safe(error: str | int) -> str | int:
+ """
Review Comment:
`ruff` fails here with `D200 One-line docstring should fit on one line`,
which is what the CI Static checks job is failing on. `main` passes this hook,
so it comes from this PR.
```python
def make_jsonb_safe(error: str | int) -> str | int:
"""Strip characters that cannot be stored in a Postgres ``jsonb`` column
from error text."""
```
As a first-time contributor you may not have the hooks installed yet — `uv
tool install prek && prek install` will run these locally before each commit
and catch this class of failure before pushing. See [static code
checks](https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst).
##########
providers/databricks/src/airflow/providers/databricks/utils/databricks.py:
##########
@@ -17,9 +17,22 @@
# under the License.
from __future__ import annotations
+import re
+
from airflow.providers.common.compat.sdk import AirflowException, XComArg
from airflow.providers.databricks.hooks.databricks import DatabricksHook,
RunState
+_JSONB_INVALID_CHARS = re.compile(r"[\x00\ud800-\udfff]")
+
+
+def make_jsonb_safe(error: str | int) -> str | int:
Review Comment:
Minor, non-blocking: the annotation `error: str | int` is narrower than what
the function actually accepts — it passes through any non-string unchanged, and
callers here only ever pass `run_output["error"]` or `state_message`, both of
which are strings in practice. Since the guard exists to be defensive about
whatever Databricks returns, `error: Any -> Any` (or just `str` if you want to
assert the contract) would describe the behaviour more honestly than `str |
int`, which suggests `int` is a meaningful case when it is only incidental.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
