You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2023/01/07 01:13:07 UTC
[GitHub] [airflow] hussein-awala opened a new pull request, #28777: Fix dag run conf encoding with non-JSON serializable values
hussein-awala opened a new pull request, #28777:
URL: https://github.com/apache/airflow/pull/28777
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
closes: #28772
---
**^ Add meaningful description above**
This PR creates a custom json encoder class to encode non-JSON serializable values added to the dag run conf dictionary.
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bolkedebruin commented on a diff in pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by GitBox <gi...@apache.org>.
bolkedebruin commented on code in PR #28777:
URL: https://github.com/apache/airflow/pull/28777#discussion_r1065535999
##########
airflow/utils/json.py:
##########
@@ -113,5 +113,20 @@ def orm_object_hook(dct: dict) -> object:
return deserialize(dct, False)
+class DagRunConfEncoder(WebEncoder):
Review Comment:
Make this part of WebEncoder rather than creating another one
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28777:
URL: https://github.com/apache/airflow/pull/28777#discussion_r1064248601
##########
airflow/www/utils.py:
##########
@@ -133,13 +133,25 @@ def get_mapped_summary(parent_instance, task_instances):
def get_dag_run_conf(dag_run_conf: Any) -> tuple[str | None, bool]:
+ class DagRunConfEncoder(json.JSONEncoder):
+ def default(self, obj):
+ if isinstance(obj, bytes):
+ try:
+ return obj.decode()
+ except Exception:
+ return str(obj)
+ try:
+ return json.JSONEncoder.default(self, obj)
+ except Exception:
+ return str(obj)
Review Comment:
I just checked and indeed this function is only used in `views.py`. However since there’s no way to tell whether it’s also used by user code somewhere (backward compatibility concerns), it’s probably better to make the function signature something like this:
```python
def get_dag_run_conf(
dag_run_conf: Any,
*,
json_encoder: Type[json.JSONEncoder] = json.JSONEncoder,
) -> tuple[str | None, bool]:
...
```
and we can move `DagRunConfEncoder` to `vies.py` as a private class.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] hussein-awala commented on a diff in pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by GitBox <gi...@apache.org>.
hussein-awala commented on code in PR #28777:
URL: https://github.com/apache/airflow/pull/28777#discussion_r1064173784
##########
airflow/www/utils.py:
##########
@@ -133,13 +133,25 @@ def get_mapped_summary(parent_instance, task_instances):
def get_dag_run_conf(dag_run_conf: Any) -> tuple[str | None, bool]:
+ class DagRunConfEncoder(json.JSONEncoder):
+ def default(self, obj):
+ if isinstance(obj, bytes):
+ try:
+ return obj.decode()
+ except Exception:
+ return str(obj)
+ try:
+ return json.JSONEncoder.default(self, obj)
+ except Exception:
+ return str(obj)
Review Comment:
This method is only used to show dag run conf dictionary content in Airflow Webserver, but the same content is stored in the database as `PickleType` without any type conversion, and it read and deserialized when we call `dag_run.conf`.
As long as there are no problems with handling the different types in the conf dict, I think converting binary types to str can solve the UI problem.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] hussein-awala commented on a diff in pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by GitBox <gi...@apache.org>.
hussein-awala commented on code in PR #28777:
URL: https://github.com/apache/airflow/pull/28777#discussion_r1074043280
##########
airflow/utils/json.py:
##########
@@ -59,7 +59,11 @@ def default(self, o: Any) -> Any:
data = serialize(o)
if isinstance(data, dict) and DATA in data:
return data[DATA]
-
+ if isinstance(o, bytes):
+ try:
+ return o.decode()
+ except Exception:
Review Comment:
I think it's `UnicodeDecodeError`
```suggestion
except UnicodeDecodeError:
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28777:
URL: https://github.com/apache/airflow/pull/28777#discussion_r1070956712
##########
airflow/utils/json.py:
##########
@@ -71,7 +75,7 @@ def default(self, o: Any) -> Any:
return data[DATA]
return data
except TypeError:
- raise
+ return o.__repr__()
Review Comment:
Also here
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] hussein-awala commented on a diff in pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by GitBox <gi...@apache.org>.
hussein-awala commented on code in PR #28777:
URL: https://github.com/apache/airflow/pull/28777#discussion_r1064216151
##########
airflow/www/utils.py:
##########
@@ -133,13 +133,25 @@ def get_mapped_summary(parent_instance, task_instances):
def get_dag_run_conf(dag_run_conf: Any) -> tuple[str | None, bool]:
+ class DagRunConfEncoder(json.JSONEncoder):
+ def default(self, obj):
+ if isinstance(obj, bytes):
+ try:
+ return obj.decode()
+ except Exception:
+ return str(obj)
+ try:
+ return json.JSONEncoder.default(self, obj)
+ except Exception:
+ return str(obj)
Review Comment:
Actually It's possible using the Airflow python API (the method [DAG.create_dagrun](https://airflow.apache.org/docs/apache-airflow/stable/_api/airflow/models/dag/index.html#airflow.models.dag.DAG.create_dagrun)):
```python
dag_bag = DagBag(read_dags_from_db=True)
dag_bag.collect_dags_from_db()
dag = dag_bag.get_dag("dag_id")
dag.create_dagrun(
run_id="run_id",
run_type=DagRunType.MANUAL,
execution_date=run_start_date,
state=State.QUEUED,
external_trigger=True,
data_interval=(run_start_date, run_end_date),
conf = {<str key>: <any value>}
)
```
And we use this method frequently in the [custom plugins](https://airflow.apache.org/docs/apache-airflow/stable/plugins.html). This part is public and documented.
Also we can update the run conf from a task, and the updated fields are accessible from the other tasks ([ex](https://github.com/apache/airflow/issues/28772))
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28777:
URL: https://github.com/apache/airflow/pull/28777#discussion_r1073178511
##########
airflow/utils/json.py:
##########
@@ -59,7 +59,11 @@ def default(self, o: Any) -> Any:
data = serialize(o)
if isinstance(data, dict) and DATA in data:
return data[DATA]
-
+ if isinstance(o, bytes):
+ try:
+ return o.decode()
+ except Exception:
Review Comment:
```suggestion
except UnicodeDecodeException:
```
?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28777:
URL: https://github.com/apache/airflow/pull/28777#discussion_r1073178788
##########
airflow/utils/json.py:
##########
@@ -71,7 +75,7 @@ def default(self, o: Any) -> Any:
return data[DATA]
return data
except TypeError:
- raise
+ return repr(o)
Review Comment:
Is this one related?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #28777:
URL: https://github.com/apache/airflow/pull/28777#discussion_r1102509392
##########
airflow/utils/json.py:
##########
@@ -71,7 +75,7 @@ def default(self, o: Any) -> Any:
return data[DATA]
return data
except TypeError:
- raise
+ return repr(o)
Review Comment:
Makes sense. We should probably mention this is the encoder’s docstring.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] hussein-awala commented on a diff in pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by GitBox <gi...@apache.org>.
hussein-awala commented on code in PR #28777:
URL: https://github.com/apache/airflow/pull/28777#discussion_r1074037887
##########
airflow/utils/json.py:
##########
@@ -71,7 +75,7 @@ def default(self, o: Any) -> Any:
return data[DATA]
return data
except TypeError:
- raise
+ return repr(o)
Review Comment:
I'm not sure that returning `repr(o)` whatever the o type is a good idea, but I'm trying to generalize this, where we cannot handle all the types especially the user custom classes, so instead of raising an exception, we can return the value as string since it's just to show it in the UI.
What do you think?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #28777:
URL: https://github.com/apache/airflow/pull/28777#discussion_r1102508940
##########
airflow/utils/json.py:
##########
@@ -59,7 +59,11 @@ def default(self, o: Any) -> Any:
data = serialize(o)
if isinstance(data, dict) and DATA in data:
return data[DATA]
-
+ if isinstance(o, bytes):
+ try:
+ return o.decode()
+ except UnicodeDecodeError:
+ return repr(o)
Review Comment:
An alternative would be to use `o.decode("unicode_escape")`, which renders undecodable characters as `\uXXXX`. This is probably better than leaking the `b"..."` format.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Taragolis commented on a diff in pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by GitBox <gi...@apache.org>.
Taragolis commented on code in PR #28777:
URL: https://github.com/apache/airflow/pull/28777#discussion_r1064190069
##########
airflow/www/utils.py:
##########
@@ -133,13 +133,25 @@ def get_mapped_summary(parent_instance, task_instances):
def get_dag_run_conf(dag_run_conf: Any) -> tuple[str | None, bool]:
+ class DagRunConfEncoder(json.JSONEncoder):
+ def default(self, obj):
+ if isinstance(obj, bytes):
+ try:
+ return obj.decode()
+ except Exception:
+ return str(obj)
+ try:
+ return json.JSONEncoder.default(self, obj)
+ except Exception:
+ return str(obj)
Review Comment:
Another concern I have no idea how to add non-JSON Serializable objects into the `conf` by use public parts of Airflow and should we encourage the use of undocumented features
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Taragolis commented on a diff in pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by GitBox <gi...@apache.org>.
Taragolis commented on code in PR #28777:
URL: https://github.com/apache/airflow/pull/28777#discussion_r1064227851
##########
airflow/www/utils.py:
##########
@@ -133,13 +133,25 @@ def get_mapped_summary(parent_instance, task_instances):
def get_dag_run_conf(dag_run_conf: Any) -> tuple[str | None, bool]:
+ class DagRunConfEncoder(json.JSONEncoder):
+ def default(self, obj):
+ if isinstance(obj, bytes):
+ try:
+ return obj.decode()
+ except Exception:
+ return str(obj)
+ try:
+ return json.JSONEncoder.default(self, obj)
+ except Exception:
+ return str(obj)
Review Comment:
Well, `DagBag` it is definitely not a part of public interface and intend by use in plugins and by users as well as manually call `DAG.create_dagrun` outside of the tests, the only one operator directly use it is SubDagOperator, which deprecated and will be remove in Airflow 3
The general usage of the DAG is create DAG object and assign it by one of the [available way](https://airflow.apache.org/docs/apache-airflow/stable/concepts/dags.html#dag-assignment).
For run DAG user could use REST API/CLI/TriggerDagRunOperator/Scheduler, all other ways is not documented and definitely not recommended. I found same issue when TriggerDagRunOperator supported send non-json serialisable data to `conf`, and solution was ban this ability, see https://github.com/apache/airflow/issues/13414
Not all methods/functions/classes have `_` in the beginning but it doesn't mean that this methods could directly call by programmatic way. Best example is `execute` method of `BaseOperator` which only intend to use by Scheduler/Executor.
There is also PR https://github.com/apache/airflow/pull/28300 which prepare info about public Airflow interface, I would suggest clarifying about usage some part of airflow (DagBag, DAG.create_dagrun) in that PR because it might help to cover more grey zones.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28777:
URL: https://github.com/apache/airflow/pull/28777#discussion_r1064247537
##########
airflow/www/utils.py:
##########
@@ -133,13 +133,25 @@ def get_mapped_summary(parent_instance, task_instances):
def get_dag_run_conf(dag_run_conf: Any) -> tuple[str | None, bool]:
+ class DagRunConfEncoder(json.JSONEncoder):
+ def default(self, obj):
+ if isinstance(obj, bytes):
+ try:
+ return obj.decode()
+ except Exception:
+ return str(obj)
+ try:
+ return json.JSONEncoder.default(self, obj)
+ except Exception:
+ return str(obj)
Review Comment:
Just to comment, if you _have_ to stringify things for display, `repr` is better than `str`.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #28777:
URL: https://github.com/apache/airflow/pull/28777#discussion_r1103964681
##########
airflow/utils/json.py:
##########
@@ -44,7 +44,12 @@ def loads(self, s: str | bytes, **kwargs):
class WebEncoder(json.JSONEncoder):
- """This encodes values into a web understandable format. There is no deserializer"""
+ """
+ This encodes values into a web understandable format. There is no deserializer.
+ It parses datetime, dates, Decimal and bytes. In order to parse the custom
+ classes and the other types, and since it's just to show the result in the UI,
+ we return repr(object) for everything else.
+ """
Review Comment:
```suggestion
"""This encodes values into a web understandable format. There is no deserializer.
This parses datetime, dates, Decimal and bytes. In order to parse the custom
classes and the other types, and since it's just to show the result in the UI,
we return repr(object) for everything else.
"""
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] hussein-awala commented on pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by "hussein-awala (via GitHub)" <gi...@apache.org>.
hussein-awala commented on PR #28777:
URL: https://github.com/apache/airflow/pull/28777#issuecomment-1489419442
@Taragolis @bolkedebruin I think we need a second approve to merge this PR, can you please review it?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #28777:
URL: https://github.com/apache/airflow/pull/28777
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bolkedebruin commented on a diff in pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by GitBox <gi...@apache.org>.
bolkedebruin commented on code in PR #28777:
URL: https://github.com/apache/airflow/pull/28777#discussion_r1065536450
##########
airflow/utils/json.py:
##########
@@ -113,5 +113,20 @@ def orm_object_hook(dct: dict) -> object:
return deserialize(dct, False)
+class DagRunConfEncoder(WebEncoder):
+ """This encodes DagRunConf values into a web understandable format. There is no deserializer"""
+
+ def default(self, obj):
+ try:
+ return super().default(obj)
+ except Exception:
+ if isinstance(obj, bytes):
Review Comment:
Can you be more sure of what this is so a try/except isnt required?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bolkedebruin commented on pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by GitBox <gi...@apache.org>.
bolkedebruin commented on PR #28777:
URL: https://github.com/apache/airflow/pull/28777#issuecomment-1376117699
I've just merged all encoders into (almost) one. Let's not add another one and extend the one used for the Web if required (There is a WebEncoder).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Taragolis commented on a diff in pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by GitBox <gi...@apache.org>.
Taragolis commented on code in PR #28777:
URL: https://github.com/apache/airflow/pull/28777#discussion_r1063989966
##########
airflow/www/utils.py:
##########
@@ -133,13 +133,25 @@ def get_mapped_summary(parent_instance, task_instances):
def get_dag_run_conf(dag_run_conf: Any) -> tuple[str | None, bool]:
+ class DagRunConfEncoder(json.JSONEncoder):
+ def default(self, obj):
+ if isinstance(obj, bytes):
+ try:
+ return obj.decode()
+ except Exception:
+ return str(obj)
+ try:
+ return json.JSONEncoder.default(self, obj)
+ except Exception:
+ return str(obj)
Review Comment:
I'm not sure is this a good idea to convert everything to string representation, personally I have no idea how users might use this values later in case of binary data (or some specific classes)
```python
Python 3.9.10 (main, Feb 25 2022, 16:54:01)
Type 'copyright', 'credits' or 'license' for more information
IPython 8.7.0 -- An enhanced Interactive Python. Type '?' for help.
PyDev console: using IPython 8.7.0
Python 3.9.10 (main, Feb 25 2022, 16:54:01)
[Clang 13.0.0 (clang-1300.0.29.30)] on darwin
import pickle
o = pickle.dumps({"a": 1, "b": 2})
str(o)
Out[4]: "b'\\x80\\x04\\x95\\x11\\x00\\x00\\x00\\x00\\x00\\x00\\x00}\\x94(\\x8c\\x01a\\x94K\\x01\\x8c\\x01b\\x94K\\x02u.'"
```
It's look like we hide one error and postpone it into the later interaction.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #28777: Fix dag run conf encoding with non-JSON serializable values
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28777:
URL: https://github.com/apache/airflow/pull/28777#discussion_r1068974814
##########
airflow/utils/json.py:
##########
@@ -59,7 +59,11 @@ def default(self, o: Any) -> Any:
data = serialize(o)
if isinstance(data, dict) and DATA in data:
return data[DATA]
-
+ if isinstance(o, bytes):
+ try:
+ return o.decode()
+ except Exception:
+ return o.__repr__()
Review Comment:
```suggestion
return repr(o)
```
Same for the one below
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org