SameerMesiah97 commented on code in PR #63210:
URL: https://github.com/apache/airflow/pull/63210#discussion_r2937176445
##########
providers/common/sql/tests/unit/common/sql/operators/test_sql.py:
##########
@@ -605,6 +605,21 @@ def test_sql_check_partition_clause_templating(self,
conn_id):
finally:
hook.run(["DROP TABLE employees"])
+ def test_sql_check_accept_none_true(self, monkeypatch):
+ """Validate that empty table does not fail when accept_none=True."""
+ records = []
+ operator = self._construct_operator(monkeypatch, self.checks, records)
+ operator.accept_none = True
Review Comment:
I understand that you are not passing the argument for `_construct_operator`
in the constructor because it does not accept it expcility or have `**kwargs`
in its signature but why not just call `SQLTableCheckOperator` directly?
##########
providers/common/sql/src/airflow/providers/common/sql/operators/sql.py:
##########
@@ -738,20 +739,32 @@ def __init__(
partition_clause: str | None = None,
conn_id: str | None = None,
database: str | None = None,
+ accept_none: bool = False,
**kwargs,
):
super().__init__(conn_id=conn_id, database=database, **kwargs)
self.table = table
self.checks = checks
self.partition_clause = _initialize_partition_clause(partition_clause)
+ self.accept_none = accept_none
self.sql = f"SELECT check_name, check_result FROM
({self._generate_sql_query()}) AS check_table"
def execute(self, context: Context):
hook = self.get_db_hook()
records = hook.get_records(self.sql)
if not records:
Review Comment:
How confident are you that records will be falsy when the table is emtpy?
Granted, I am not too familiar with this specific provider but after tracing
the full code-path down to the hook and running a few test queries in
snowflake, it seems like that records can not be falsy (I am assuming you are
expecting an empty list here). So this renders everything below `if not
records` inert. Have you tried testing this using a real DAG and a live DB?
##########
providers/common/sql/tests/unit/common/sql/operators/test_sql.py:
##########
@@ -605,6 +605,21 @@ def test_sql_check_partition_clause_templating(self,
conn_id):
finally:
hook.run(["DROP TABLE employees"])
+ def test_sql_check_accept_none_true(self, monkeypatch):
+ """Validate that empty table does not fail when accept_none=True."""
+ records = []
+ operator = self._construct_operator(monkeypatch, self.checks, records)
+ operator.accept_none = True
+ operator.execute(context=MagicMock())
Review Comment:
There are no asserts for the positive scenario. You may have to do something
similar to `test_pass_all_checks_check` over here i.e. asserting
`operator.checks`.
--
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]
