potiuk commented on code in PR #30255:
URL: https://github.com/apache/airflow/pull/30255#discussion_r1147263587
##########
airflow/task/task_runner/base_task_runner.py:
##########
@@ -46,18 +49,32 @@ class BaseTaskRunner(LoggingMixin):
Invoke the `airflow tasks run` command with raw mode enabled in a
subprocess.
- :param local_task_job: The local task job associated with running the
- associated task instance.
+ :param base_job: The job associated with running the associated task
instance. The job_runner for it
+ should be LocalTaskJobRunner
+ :param job_runner: The job runner associated with running the associated
task instance. This parameter
+ is only used in case base_job is BaseJobPydantic instance - i.e. in
case of db-less mode of
+ Airflow workers. This means that in case you have your own custom
task runner, you should
+ adopt it to handle both BaseJob (with runner being part of the job)
and
+ BaseJobPydantic (with runner passed as a separate parameter). For
non-db-less-mode, the
+ runner is always part of the job, so custom task runners should be
backwards-compatible.
"""
- def __init__(self, local_task_job):
+ def __init__(self, base_job: BaseJob | BaseJobPydantic, job_runner:
LocalTaskJobRunner | None = None):
+ if hasattr(base_job, "job_runner") and job_runner is None:
Review Comment:
The problem was with BaseJobPydantic. I wanted to keep it a "pure"
data-class - i.e. holding only things that can be serialized and sent over the
wire (+ possibly has methods that we could call - but that is another change).
But your comment made me think that likely yes - instead of addig a
job_runner attribute we could have `get_job_runner()` method and in case of
BaseJobPydantic it could be @cached method that would return the runner based
on other properties of the BaseJob, so I **think** we can simplify it indeed.
--
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]
