[DPTOOLS-604] Remove deprecate warning #59
Conversation
| @@ -36,9 +36,9 @@ | |||
|
|
|||
| # show Airflow's deprecation warnings | |||
| warnings.filterwarnings( | |||
| action='default', category=DeprecationWarning, module='airflow') | |||
| action='ignore', category=DeprecationWarning, module='airflow') | |||
There was a problem hiding this comment.
Rather than ignore these, could we just fix them?
There was a problem hiding this comment.
The warning comes from here(https://github.com/lyft/incubator-airflow/blob/data-platform-upgrade/airflow/models.py#L1977-L1985) which comes from users passing un-named arguments to operators. The original pr checked in to upstream airflow in 2016 and we still saw many operators passing un-named arguments.
I think the fix could be :
option 1: remove the warning in our repo(remove https://github.com/lyft/incubator-airflow/blob/data-platform-upgrade/airflow/models.py#L1977-L1985)
option 2: similar to this pr
option 3: fix all the existing dag operator usage and prevent them from passing un-named arguments.
option 4: check with the upstream airflow community and see if we still want to disable using *args and *kargs as the original pr is in 2016 and targeted for airflow 2.0(no timeline). If yes, we could start with fixing the upstream usage first and then bring it back to lyft repo.
I am fine with option 1 and 2, but option 3, 4 seems to be non-trivial. What do you think?
There was a problem hiding this comment.
All the code that is passing un-named args are in the etl code base, not part of upstream Airflow, right? If so, I'd be in favor of actually fixing them. I worry that hiding the warnings will hide real bugs from users writing DAGs.
There was a problem hiding this comment.
I think it involves first fixing all the existing operators(e.g https://github.com/lyft/etl/blob/master/lib/lyftdata/airflow/plugins/operators/anodot_operator.py), then fix all the usage for the dags which use kwargs in operators. And we can't fix in upstream(e.g remove kwargs and args in BaseOperator), we can't prevent users to continue using kwargs even after we fix all the issues. Let me send out an email to upstream alias and check if it is still on the roadmap to remove kwargs in BaseOperator.
And I am not sure how this message could user debug their DAG as well. One thing I worry about your approach is that it will touch many dags which could cause unknown dag failure as we are not familiar with the dags.
|
👍 |
Not sure why we have these warning in airflow(apache#1285).
But we won't show it in our lyft repo:)