dstandish commented on code in PR #41904:
URL: https://github.com/apache/airflow/pull/41904#discussion_r1739391750
##########
airflow/providers/celery/executors/celery_kubernetes_executor.py:
##########
@@ -30,18 +31,21 @@
raise AirflowOptionalProviderFeatureException(e)
-from airflow.utils.log.logging_mixin import LoggingMixin
from airflow.utils.providers_configuration_loader import
providers_configuration_loaded
if TYPE_CHECKING:
from airflow.callbacks.base_callback_sink import BaseCallbackSink
from airflow.callbacks.callback_requests import CallbackRequest
- from airflow.executors.base_executor import CommandType,
EventBufferValueType, QueuedTaskInstanceType
+ from airflow.executors.base_executor import (
+ CommandType,
+ EventBufferValueType,
+ QueuedTaskInstanceType,
+ )
from airflow.models.taskinstance import SimpleTaskInstance, TaskInstance
from airflow.models.taskinstancekey import TaskInstanceKey
-class CeleryKubernetesExecutor(LoggingMixin):
+class CeleryKubernetesExecutor(BaseExecutor):
Review Comment:
I don't really have a strong opinion.
That said it does not seem right to not have an executor be an instance of
BaseExecutor. I strongly suspect that the reason this was not done in the
first place was mypy. There was actually a bug where you [could not override
an attribute with a property even if you implemented a
setter](https://github.com/python/mypy/issues/9190). I'm not sure if that's
what tripped up the orig author, but in any case, that issue has been resolved.
But it doesn't really matter to me and if you think it's best not have these
things inherit then I don't mind.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
