shahar1 commented on code in PR #70326:
URL: https://github.com/apache/airflow/pull/70326#discussion_r3645889100
##########
providers/weaviate/src/airflow/providers/weaviate/operators/weaviate.py:
##########
@@ -75,15 +75,16 @@ def __init__(
self.input_data = input_data
self.hook_params = hook_params or {}
- if self.input_data is None:
- raise TypeError("input_data is required")
-
@cached_property
def hook(self) -> WeaviateHook:
"""Return an instance of the WeaviateHook."""
return WeaviateHook(conn_id=self.conn_id, **self.hook_params)
def execute(self, context: Context) -> None:
+ # input_data is a template field; validate it after rendering rather
than in __init__,
+ # where the check would run against the un-rendered value.
Review Comment:
Please remove these comments (they are not too useful)
##########
providers/weaviate/src/airflow/providers/weaviate/operators/weaviate.py:
##########
@@ -75,15 +75,16 @@ def __init__(
self.input_data = input_data
self.hook_params = hook_params or {}
- if self.input_data is None:
- raise TypeError("input_data is required")
-
@cached_property
def hook(self) -> WeaviateHook:
"""Return an instance of the WeaviateHook."""
return WeaviateHook(conn_id=self.conn_id, **self.hook_params)
def execute(self, context: Context) -> None:
+ # input_data is a template field; validate it after rendering rather
than in __init__,
+ # where the check would run against the un-rendered value.
+ if self.input_data is None:
+ raise TypeError("input_data is required")
Review Comment:
The direction is right, but for an `is None` check the "validates the
un-rendered value" rationale doesn't apply: a templated `input_data` is a Jinja
string or `XComArg` (never `None` at construction), and a literal `None`
renders to `None`. The old `__init__` check had no false positives - moving it
to `execute()` just delays the common mistake (omitting `input_data`, silent
due to the `= None` default) from a parse-time import error to a runtime task
failure that burns retries.
The `= None` default is vestigial from the removed input_json alternative
(#44745). Suggest dropping it, i.e.,
```
input_data: list[dict[str, Any]] | pd.DataFrame,
```
That restores the parse-time `TypeError` via Python itself, still passes the
hook (signature defaults are excluded from its scan), and lets mypy flag
input_data=None. Keep the new `execute()` check - it's a real improvement for
values that render to `None`, which previously crashed as 'NoneType' object is
not iterable inside `hook.batch_data`. Your new test passes unchanged under
this signature.
---
Drafted-by: Claude Code (Fable 5); reviewed by me before posting
--
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]
