kacpermuda commented on code in PR #38408:
URL: https://github.com/apache/airflow/pull/38408#discussion_r1537239202
##########
airflow/providers/google/cloud/operators/bigquery.py:
##########
@@ -322,8 +322,26 @@ def execute(self, context: Context):
),
method_name="execute_complete",
)
+ self._handle_job_error(job)
+ # job.result() returns a RowIterator. Mypy expects an instance of
SupportsNext[Any] for
+ # the next() call which the RowIterator does not resemble to.
Hence, ignore the arg-type error.
+ records = next(job.result()) # type: ignore[arg-type]
Review Comment:
Yes, `next()` returns only a single row, and we are only checking the first
row. `validate_records` expect a single row (even though the name of the
argument is `records`, plural). This is because I was recreating what's
happening in the
[Trigger](https://github.com/apache/airflow/blob/420709962ec864fa62b6e1b6c2f723a2c14cb751/airflow/providers/google/cloud/triggers/bigquery.py#L165)
itself that checks only the first row (but single row is returned as
`event["records"]`). I was wondering why it happens as in the execute_complete
uses the `all(records):` that does not check much in that case, but that's
probably for another issue. For now, i think when not deferred but in
deferrable mode, we should recreate what's happening in the trigger to maintain
the same behaviour.
Also, this `next()` part is copied from BigQueryValueCheckOperator's
[check](https://github.com/apache/airflow/blob/420709962ec864fa62b6e1b6c2f723a2c14cb751/airflow/providers/google/cloud/operators/bigquery.py#L454),
so if it's wrong, it should be changed there too.
Maybe that's not the correct attitude, let me know if there is a better way
to do 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
