You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Jarek Potiuk (Jira)" <ji...@apache.org> on 2019/12/01 08:30:00 UTC
[jira] [Commented] (AIRFLOW-3407) BaseOperator and LoggingMixin do
not call super().__init__
[ https://issues.apache.org/jira/browse/AIRFLOW-3407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16985500#comment-16985500 ]
Jarek Potiuk commented on AIRFLOW-3407:
---------------------------------------
I think this is something we should possibly discuss and agree how we want BaseOperator to behave in more complex cases of multiple-inheritance hierarchy.
Using super().__init__(...) in multiple inheritance cases is far from simple (and there are different opinions on what's the best approach) and I think it's really a matter of how extensible and mixin-able we want BaseOperator to be.
The absolute "best" practice I follow in this case is "Avoid multiple inheritance at all cost". There are better ways of extending classes (composition over inheritance) and I usually prefer the former.
I think BaseOperator does not necessary have to agree on passing some unknown arguments to unknown classes. And I think someone else also had the same thought: we even have this warning few lines into the BaseOperator's __init__:
'Invalid arguments were passed to \{c} (task_id: \{t}). '
'Support for passing such arguments will be dropped in '
'future. Invalid arguments were:' ...
And we even have exception thrown in case conf.getboolean('operators', 'ALLOW_ILLEGAL_ARGUMENTS') is False.
So passing the **args and **kwargs further kind of goes against the future. Adding it right now is not a good Idea.
WDYT? What are the use cases you think passing *args, **kwargs might be useful at? Should we allow to extend BaseOperator at all with some unknown Mixins? I have a feeling that BaseOperator is such a "Base" class that we should always make it last in the MRO order (well LoggingMixin will be last eventually).
> BaseOperator and LoggingMixin do not call super().__init__
> ----------------------------------------------------------
>
> Key: AIRFLOW-3407
> URL: https://issues.apache.org/jira/browse/AIRFLOW-3407
> Project: Apache Airflow
> Issue Type: Bug
> Components: operators
> Affects Versions: 1.10.1
> Reporter: adam hitchcock
> Assignee: Chao-Han Tsai
> Priority: Major
>
> The {{BaseOperator}} is not necessarily the last class in the MRO; usually it is best practice to always call {{super().__init__(*args, **kwargs)}}
> to make sure that every class gets it chance to {{__init__}}.
> Is there a specific reason {{BaseOperator}} doesn't call super?
--
This message was sent by Atlassian Jira
(v8.3.4#803005)