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)