potiuk commented on code in PR #30166:
URL: https://github.com/apache/airflow/pull/30166#discussion_r1179794212
##########
airflow/providers/common/sql/operators/sql.py:
##########
@@ -585,41 +622,70 @@ def __init__(
partition_clause: str | None = None,
conn_id: str | None = None,
database: str | None = None,
+ accept_none: bool = True,
Review Comment:
Sure we can do it this way, but I find it a bit odd to add new parameter and
deprecate it immediately. Somehow like - we are adding this new feature there,
but BTW it's deprecated.
If we deprecate it in easy way, there will be no way to distinguish if user
deliberately passed False, or it was the default one.
False is immutable so you cannot use the technique from
https://stackoverflow.com/questions/11251119/is-there-any-way-to-know-if-the-value-of-an-argument-is-the-default-vs-user-spe
where you could compare identity of passed object. As far as I know. If you
make `accept_none=False` the default. for all practical purposes (other than
introspecting the source code from the call stack) -
`SQLTableCheckOperator(...., accept_none=False)` is indistinguishable from
`SQLTableCheckOperator(...., ). Which means that if we deprecate
accept_none=False, there will be no way to pass False without a Deprecation
warnings - i.e. user who would like to use False will have no way to remove the
warning.
You could potentially pass a sentinel object and treat it as False (and
deprecate), but that's really makes things like that complex and ugly (also
confusing user):
```
SQLTableCheckOperator(, accept_none: bool | Literal["TREAT_ME_AS_FALSE"] =
"TREAT_ME_AS_FALSE")
```
But if you think it's likely that someone would fall into backwards
compatibility there, and it's worth it - yeah. it could be done.
--
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]
