jedcunningham commented on code in PR #29055:
URL: https://github.com/apache/airflow/pull/29055#discussion_r1265967680
##########
airflow/executors/executor_loader.py:
##########
@@ -148,14 +154,16 @@ def _import_and_validate(path: str) -> type[BaseExecutor]:
return _import_and_validate(executor_name), ConnectorSource.CUSTOM_PATH
@classmethod
- def import_default_executor_cls(cls) -> tuple[type[BaseExecutor],
ConnectorSource]:
+ def import_default_executor_cls(cls, validate=True) ->
tuple[type[BaseExecutor], ConnectorSource]:
Review Comment:
```suggestion
def import_default_executor_cls(cls, validate: bool = True) ->
tuple[type[BaseExecutor], ConnectorSource]:
```
##########
airflow/executors/celery_kubernetes_executor.py:
##########
@@ -231,3 +231,7 @@ def send_callback(self, request: CallbackRequest) -> None:
if not self.callback_sink:
raise ValueError("Callback sink is not ready.")
self.callback_sink.send(request)
+
+ @staticmethod
+ def get_cli_commands() -> list:
+ return []
Review Comment:
Hmm, not sure this is right? Don't we want to expose the celery and k8s cli
still?
##########
airflow/executors/executor_loader.py:
##########
@@ -184,6 +192,31 @@ def validate_database_executor_compatibility(cls,
executor: type[BaseExecutor])
if engine.dialect.name == "sqlite":
raise AirflowConfigException(f"error: cannot use SQLite with the
{executor.__name__}")
+ @classmethod
+ def import_all_executor_classes(cls, validate=True) ->
list[type[BaseExecutor]]:
Review Comment:
```suggestion
def import_all_executor_classes(cls, *, validate: bool = True) ->
list[type[BaseExecutor]]:
```
kwarg only probably makes sense here?
##########
airflow/executors/executor_loader.py:
##########
@@ -116,18 +116,24 @@ def load_executor(cls, executor_name: str) ->
BaseExecutor:
return executor_cls()
@classmethod
- def import_executor_cls(cls, executor_name: str) ->
tuple[type[BaseExecutor], ConnectorSource]:
+ def import_executor_cls(
+ cls, executor_name: str, validate=True
Review Comment:
```suggestion
cls, executor_name: str, validate: bool = True
```
##########
airflow/executors/local_kubernetes_executor.py:
##########
@@ -230,3 +230,7 @@ def send_callback(self, request: CallbackRequest) -> None:
if not self.callback_sink:
raise ValueError("Callback sink is not ready.")
self.callback_sink.send(request)
+
+ @staticmethod
+ def get_cli_commands() -> list:
+ return []
Review Comment:
Same here, this should include the k8s cli?
--
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]
