apache / airflow

Apache Airflow - A platform to programmatically author, schedule, and monitor workflows
https://airflow.apache.org/
Apache License 2.0
36.95k stars 14.26k forks source link

Error when inherit from QuboleOperator #28159

Closed yoavfeld closed 1 year ago

yoavfeld commented 1 year ago

Apache Airflow version

Other Airflow 2 version (please specify below)

What happened

airflow version: 2.3 python version 3.7.13

problem: error occured when inheriting from QuboleOperator (airflow/providers/qubole/operators/qubole.py)

callstack:

File "/home/airflow/.local/lib/python3.7/site-packages/airflow/providers/qubole/operators/qubole.py", line 304, in setattr if name in _get_template_fields(self): File "/home/airflow/.local/lib/python3.7/site-packages/airflow/providers/qubole/operators/qubole.py", line 312, in _get_template_fields templatefields = object.getattribute(class, 'template_fields') AttributeError: 'BaseOperatorMeta' object has no attribute 'template_fields'

What you think should happen instead

These changes in qubole.py file should be reverted:

https://github.com/apache/airflow/commit/33214d9326bb0bb52f06e230895f4f68fc952664#diff-63a6626367fb879a888263f6063e8aebb42d5a34e3a0f798fbb727b0a640aa56L270

https://github.com/apache/airflow/commit/33214d9326bb0bb52f06e230895f4f68fc952664#diff-63a6626367fb879a888263f6063e8aebb42d5a34e3a0f798fbb727b0a640aa56L279

How to reproduce

Creating a class that inherit from QuboleOperator. for ex:

from airflow.providers.qubole.operators.qubole import QuboleOperator

class MyQuboleOperator(QuboleOperator): def init(self): super().init()

Operating System

linux

Versions of Apache Airflow Providers

airflow=2.3 apache-airflow-providers-qubole==3.3.1

Deployment

Docker-Compose

Deployment details

No response

Anything else

No response

Are you willing to submit PR?

Code of Conduct

boring-cyborg[bot] commented 1 year ago

Thanks for opening your first issue here! Be sure to follow the issue template!

eladkal commented 1 year ago

confirmed. cc @potiuk

I'm not sure why we even need the __getattribute__ and __setattr__ ? https://github.com/apache/airflow/blob/78b8ea2f22239db3ef9976301234a66e50b47a94/airflow/providers/qubole/operators/qubole.py#L286-L299

it was added in https://github.com/apache/airflow/pull/1090/ I think we can just remove this part along with the _get_template_fields

potiuk commented 1 year ago

I don't think we should remove it. Looks like QuboleOpertor is precisely created in the way that template fields are SUPPOSED to be added when you derive from it.

This is what I understand from the comment in #1090 (and maybe @msumit still rememeber something that happened 6 years aago (!) and can confirm:

I do agree with your concerns. Actually we didn't wanted to add dozens of new operators like QuboleHiveOperator, QuboleHadoopOperator etc, so we'd to do hacks like that. If we had individual command operators like I said above, then we could adhere to simple design like the other operators have.

I believe the initial idea was that The template_fields will be overridden by a "child" class (@yoavfeld what you should try, is you should likely define template_fields in your derived operator:

class MyQuboleOperator(QuboleOperator):

    template_fields = ()

    def init(self):
        super().init()

I tihnk this should help you @yoavfeld and we should close the issue. Can you please confirm @yoavfeld that it solves your problem?

Converting into a discussion - unless we decide that this is an issue.