You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/12/15 16:43:40 UTC
[GitHub] [airflow] kazanzhy opened a new pull request #20319: Fix MyPy Errors for Qubole provider.
kazanzhy opened a new pull request #20319:
URL: https://github.com/apache/airflow/pull/20319
Fix MyPy Errors for Qubole provider.
related: #19891
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kazanzhy commented on pull request #20319: Fix MyPy Errors for Qubole provider.
Posted by GitBox <gi...@apache.org>.
kazanzhy commented on pull request #20319:
URL: https://github.com/apache/airflow/pull/20319#issuecomment-995252509
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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kazanzhy commented on a change in pull request #20319: Fix MyPy Errors for Qubole provider.
Posted by GitBox <gi...@apache.org>.
kazanzhy commented on a change in pull request #20319:
URL: https://github.com/apache/airflow/pull/20319#discussion_r770074731
##########
File path: airflow/providers/qubole/operators/qubole_check.py
##########
@@ -27,21 +27,26 @@
class _QuboleCheckOperatorMixin:
"""This is a Mixin for Qubole related check operators"""
+ kwargs: dict
+ results_parser_callable: Optional[Callable]
+
def execute(self, context=None) -> None:
"""Execute a check operation against Qubole"""
try:
self._hook_context = context
- super().execute(context=context)
+ super().execute(context=context) # type: ignore[misc]
Review comment:
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
##########
File path: airflow/providers/qubole/operators/qubole_check.py
##########
@@ -27,21 +27,26 @@
class _QuboleCheckOperatorMixin:
"""This is a Mixin for Qubole related check operators"""
+ kwargs: dict
+ results_parser_callable: Optional[Callable]
+
def execute(self, context=None) -> None:
"""Execute a check operation against Qubole"""
try:
self._hook_context = context
- super().execute(context=context)
+ super().execute(context=context) # type: ignore[misc]
Review comment:
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #20319: Fix MyPy Errors for Qubole provider.
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20319:
URL: https://github.com/apache/airflow/pull/20319#discussion_r770027301
##########
File path: airflow/providers/qubole/operators/qubole_check.py
##########
@@ -27,21 +27,26 @@
class _QuboleCheckOperatorMixin:
"""This is a Mixin for Qubole related check operators"""
+ kwargs: dict
+ results_parser_callable: Optional[Callable]
+
def execute(self, context=None) -> None:
"""Execute a check operation against Qubole"""
try:
self._hook_context = context
- super().execute(context=context)
+ super().execute(context=context) # type: ignore[misc]
Review comment:
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kazanzhy commented on pull request #20319: Fix MyPy Errors for Qubole provider.
Posted by GitBox <gi...@apache.org>.
kazanzhy commented on pull request #20319:
URL: https://github.com/apache/airflow/pull/20319#issuecomment-995165382
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kazanzhy commented on a change in pull request #20319: Fix MyPy Errors for Qubole provider.
Posted by GitBox <gi...@apache.org>.
kazanzhy commented on a change in pull request #20319:
URL: https://github.com/apache/airflow/pull/20319#discussion_r770054994
##########
File path: airflow/providers/qubole/operators/qubole_check.py
##########
@@ -27,21 +27,26 @@
class _QuboleCheckOperatorMixin:
"""This is a Mixin for Qubole related check operators"""
+ kwargs: dict
+ results_parser_callable: Optional[Callable]
+
def execute(self, context=None) -> None:
"""Execute a check operation against Qubole"""
try:
self._hook_context = context
- super().execute(context=context)
+ super().execute(context=context) # type: ignore[misc]
Review comment:
Nice solution :)
It is very similar to using Protocol but without protocol. Fortunately, it also works for my case.
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #20319: Fix MyPy Errors for Qubole provider.
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #20319:
URL: https://github.com/apache/airflow/pull/20319
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kazanzhy commented on a change in pull request #20319: Fix MyPy Errors for Qubole provider.
Posted by GitBox <gi...@apache.org>.
kazanzhy commented on a change in pull request #20319:
URL: https://github.com/apache/airflow/pull/20319#discussion_r770054994
##########
File path: airflow/providers/qubole/operators/qubole_check.py
##########
@@ -27,21 +27,26 @@
class _QuboleCheckOperatorMixin:
"""This is a Mixin for Qubole related check operators"""
+ kwargs: dict
+ results_parser_callable: Optional[Callable]
+
def execute(self, context=None) -> None:
"""Execute a check operation against Qubole"""
try:
self._hook_context = context
- super().execute(context=context)
+ super().execute(context=context) # type: ignore[misc]
Review comment:
Nice solution :)
It is very similar to using Protocol but without protocol. Fortunately, it also works for my case.
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #20319: Fix MyPy Errors for Qubole provider.
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #20319:
URL: https://github.com/apache/airflow/pull/20319#issuecomment-995201722
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 :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org