Skip to content

Conversation

@kazanzhy
Copy link
Contributor

Fix MyPy Errors for Qubole provider.

related: #19891

@kazanzhy kazanzhy mentioned this pull request Dec 15, 2021
10 tasks
@kazanzhy
Copy link
Contributor Author

One test is failed, but it is tests/providers/docker/decorators/test_docker.py::TestDockerDecorator::test_basic_docker_operator:.
Seems it is not related to Qubole

Comment on lines 33 to +37
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a better? way of handling it. I think it's a bit nicer:

def execute(self: BaseOperator, context=None) -> None:  # type: ignore[misc]

It's not as spectacular as in my case where I got rid of 5 self.* references this way, but I think it is more explicit (the ignore above has to be added silence another MyPy error "BaseOperator is not a superclass of Mixin'

https://github.com/apache/airflow/pull/20329/files#r770023071

Copy link
Contributor Author

@kazanzhy kazanzhy Dec 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice solution :)
It is very similar to using Protocol but without protocol. Unfortunately, it doesn't work for me.
In this case, BaseOperator hasn't method get_hook as well as none of the super classes have execute method.

I think might be cases where some class inherits from mixin and two other classes and these classes have such different attributes and can't be reduced to one. But it will be another story.
Thanks for the suggestion

@potiuk
Copy link
Member

potiuk commented Dec 15, 2021

Indeed - some instability of external APIs used for tests (have a number of those in my case) - but I think it would be nice to fix the Mixin problem in the way I found :)

@kazanzhy
Copy link
Contributor Author

So I see that mypy avoids using Mixins, which with the absence of Interfaces in Python, leads to very complex inheritance or code repetition. Am I right?

@potiuk potiuk added the mypy Fixing MyPy problems after bumpin MyPy to 0.990 label Dec 15, 2021
@potiuk potiuk merged commit 7d84196 into apache:main Dec 19, 2021
@kazanzhy kazanzhy deleted the fix_mypy_qubole branch December 20, 2021 11:24
@kaxil kaxil added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Jan 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) mypy Fixing MyPy problems after bumpin MyPy to 0.990

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants