potiuk commented on code in PR #70493:
URL: https://github.com/apache/airflow/pull/70493#discussion_r3682002710


##########
providers/google/src/airflow/providers/google/cloud/transfers/bigquery_to_mssql.py:
##########
@@ -81,15 +81,9 @@ def __init__(
 
             target_table_name = mssql_table
 
-        try:
-            _, dataset_id, table_id = source_project_dataset_table.split(".")
-        except ValueError:
-            raise ValueError(
-                f"Could not parse {source_project_dataset_table} as 
<project>.<dataset>.<table>"
-            ) from None
         super().__init__(
             target_table_name=target_table_name,
-            dataset_table=f"{dataset_id}.{table_id}",
+            
dataset_table="__airflow_unrendered_dataset.__airflow_unrendered_table",

Review Comment:
   I understand why this is here — `BigQueryToSqlBaseOperator.__init__` 
declares `dataset_table: str` as a required keyword and unconditionally does 
`self.dataset_id, self.table_id = dataset_table.split(".")`, so it needs 
*something* dot-separated at construction time. But a magic sentinel is a 
fragile way to satisfy that.
   
   It's observable: until `execute` runs, `self.dataset_id` / `self.table_id` 
hold `__airflow_unrendered_dataset` / `__airflow_unrendered_table`, and the 
parent builds `self.source_project_dataset_table = 
f"{project_id}.{self.dataset_id}.{self.table_id}"` from them. Anything reading 
the operator before execution — the UI, a log line, a task that fails during 
rendering — surfaces those strings to users with no explanation.
   
   The parent already accepts `dataset_id` and `table_id` as explicit optional 
parameters, so the cleaner fix is probably to give `dataset_table` a default of 
`None` in the base class and guard the split. That's a change to shared code, 
so happy for it to be a follow-up rather than this PR — but if the sentinel 
stays, please add a comment here saying it's a placeholder overwritten in 
`execute`, so nobody spends an afternoon grepping for where those strings come 
from.
   
   ---
   Drafted-by: Claude Code (Opus 5); reviewed by @potiuk 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]

Reply via email to