potiuk commented on code in PR #36491:
URL: https://github.com/apache/airflow/pull/36491#discussion_r1442976636
##########
airflow/providers/google/cloud/transfers/bigquery_to_postgres.py:
##########
@@ -36,8 +34,6 @@ class BigQueryToPostgresOperator(BigQueryToSqlBaseOperator):
:param postgres_conn_id: Reference to :ref:`postgres connection id
<howto/connection:postgres>`.
"""
- template_fields: Sequence[str] =
(*BigQueryToSqlBaseOperator.template_fields, "dataset_id", "table_id")
Review Comment:
I think we shoudl not treat it as breaking (or at least what I undertstand
it here).
I think the only scenario where it would matter is:
1) Someone creates a custom operator derived from BigQueryToPostgresOperator
2) The same someone adds new fields there "dataset_id" and "table_id"
3) And expects them to be templated.
Even if it worked previously, that was accidental and unintended and we
should treat this change as a bug-fix. If somoene adds new fields in a derived
operator it's their responsibilty to add those fields to templated fields.
While this change **might** technically break someone's implementation, IMHO
We should treat it as bugfix because:
a) it's a very low chance this will happen
b) while we are breaking **something** technically we are bringing things
back to how they were intended to work. Having those fields in this operator
was accidental not intentional
I will repeat it for as long as it sticks - SemVer and "breaking"
classification is not whether something is "technically" broken but whether our
intentions changed. If we woudl apply "breaking change" label for every change
that changes behaviour then pretty much every single bugfix is "technically"
breaking because it changes behaviour.
--
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]
