potiuk commented on code in PR #70542:
URL: https://github.com/apache/airflow/pull/70542#discussion_r3682042566
##########
providers/google/src/airflow/providers/google/cloud/transfers/gcs_to_bigquery.py:
##########
@@ -359,7 +344,28 @@ def _handle_job_error(job: BigQueryJob | UnknownJob) ->
None:
if job.error_result:
raise AirflowException(f"BigQuery job {job.job_id} failed:
{job.error_result}")
+ def _warn_on_deprecated_template_fields(self) -> None:
+ if self.src_fmt_configs:
+ warnings.warn(
+ "The 'src_fmt_configs' parameter is deprecated. Use
'extra_config' instead. "
+ "Note: 'extra_config' uses the fully-nested API structure, so
format-specific "
+ "options must be nested under their parent key "
+ "(e.g., {'parquetOptions': {'enableListInference': True}}
rather than "
+ "{'enableListInference': True}).",
+ AirflowProviderDeprecationWarning,
+ stacklevel=2,
Review Comment:
`stacklevel=2` is now vestigial, and this applies across the whole campaign
rather than just here.
When the warning fired from `__init__` it pointed at the Dag author's line,
which is what made it actionable. Emitted from a helper called by `execute`,
stacklevel 2 points at `execute` itself — worker code the user can't act on —
and there is no stack level that reaches the Dag file, because the Dag file
isn't on the stack at execution time.
The deprecation is still visible in the task log, which is arguably enough.
But since the parameter no longer does what it was there for, either dropping
it or naming the offending task in the message (`"task %s: the
'src_fmt_configs' parameter is deprecated..."`) would give users something to
grep. Worth deciding once for the campaign rather than per-PR.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
##########
providers/google/src/airflow/providers/google/cloud/transfers/gcs_to_bigquery.py:
##########
@@ -359,7 +344,28 @@ def _handle_job_error(job: BigQueryJob | UnknownJob) ->
None:
if job.error_result:
raise AirflowException(f"BigQuery job {job.job_id} failed:
{job.error_result}")
+ def _warn_on_deprecated_template_fields(self) -> None:
+ if self.src_fmt_configs:
+ warnings.warn(
+ "The 'src_fmt_configs' parameter is deprecated. Use
'extra_config' instead. "
+ "Note: 'extra_config' uses the fully-nested API structure, so
format-specific "
+ "options must be nested under their parent key "
+ "(e.g., {'parquetOptions': {'enableListInference': True}}
rather than "
+ "{'enableListInference': True}).",
+ AirflowProviderDeprecationWarning,
+ stacklevel=2,
+ )
+
def execute(self, context: Context):
+ # Template fields render after __init__, so defaults that depend on a
template field
+ # (schema_object_bucket falls back to bucket) and the src_fmt_configs
deprecation check
+ # must run here, against the rendered values.
+ if self.src_fmt_configs is None:
+ self.src_fmt_configs = {}
+ if self.schema_object_bucket is None:
Review Comment:
Worth being aware of a side effect: because these defaults are now resolved
*after* rendering, the rendered-template view in the UI records
`schema_object_bucket=None` even though the run actually used `bucket`. Someone
debugging a failed schema download will see `None` in the UI and the real
bucket in the logs.
Not a reason to go back — resolving before rendering was the bug — but if
`schema_object_bucket` is in `template_fields`, a log line at resolution time
(`"schema_object_bucket not set, defaulting to %s"`) would close the gap
cheaply.
---
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]
