dazza-codes commented on a change in pull request #7085: [AIRFLOW-6334] Use classes instead list of string in executors URL: https://github.com/apache/airflow/pull/7085#discussion_r363904383
########## File path: airflow/executors/celery_executor.py ########## @@ -22,15 +22,16 @@ import time import traceback from multiprocessing import Pool, cpu_count -from typing import Any, List, Optional, Tuple, Union +from typing import Any, Dict, List, Optional, Tuple, Union from celery import Celery, Task, states as celery_states from celery.result import AsyncResult from airflow.config_templates.default_celery import DEFAULT_CELERY_CONFIG from airflow.configuration import conf from airflow.exceptions import AirflowException -from airflow.executors.base_executor import BaseExecutor, CommandType +from airflow.executors.base_executor import BaseExecutor +from airflow.models.queue_task_run import LocalTaskJobDeferredRun from airflow.models.taskinstance import SimpleTaskInstance, TaskInstanceKeyType, TaskInstanceStateType Review comment: - if `queue_task_run` owns the new class, it implies a class name more like `QueuedTask` (and forget about `run` because that's implied anyway isn't it? And avoid the combination of `Task` with `Job` or use `QueuedJob` or `LocalQueuedJob` as a subclass of `QueuedJob`?) - `SimpleTaskInstance` is an instance of a task that could be recurring on a schedule and I wonder if the use of `Instance` and `Job` have some overlaps and how those terms related to scheduled tasks, task instances and task retries and whether a `job` is just one of the task-instance tries or something (best guess at how the complex concepts get chained together) ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
