Joffreybvn commented on code in PR #36205:
URL: https://github.com/apache/airflow/pull/36205#discussion_r1428246056
##########
airflow/providers/databricks/hooks/databricks_sql.py:
##########
@@ -241,13 +258,17 @@ def run(
else:
return results
- @staticmethod
- def _make_serializable(result):
- """Transform the databricks Row objects into JSON-serializable
lists."""
- if isinstance(result, list):
- return [list(row) for row in result]
- elif isinstance(result, Row):
- return list(result)
+ def _make_serializable(self, result):
+ """Transform the databricks Row objects into serializable
namedtuple."""
+ if self.return_serializable:
+ columns: list[str] | None = None
+ if isinstance(result, list):
+ columns = result[0].__fields__
+ row_object = namedtuple("Row", columns)
+ return [row_object(*row) for row in result]
+ elif isinstance(result, Row):
+ columns = result.__fields__
+ return namedtuple("Row", columns)(*result)
return result
Review Comment:
Not a strong opinion, but I think we do want that:
The `result` input of this function is the output of the
[`handler()`](https://github.com/apache/airflow/blob/280b8b30061af2dec60ac83f9fafb92b3f0663c1/airflow/providers/databricks/hooks/databricks_sql.py#L243C58-L243C65)
method. Which can be one of the default ones of Airflow (doing simply
*fetchone()* or *fetchall()*), but also a totally custom handler injected via
the `handler` parameter. Maybe the user want to do something else/something
more after fetching, which returns something else than a Row / list[Row] ? Even
if it's not the right place to transform the data IMO, blocking anything else
than Row/list[Row] can potentially break workflows.
I can improve the isinstance on the list to check if this list contains Row
object. And otherwise, the list get ignored too and returned as-is.
--
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]
