ferruzzi commented on code in PR #38054:
URL: https://github.com/apache/airflow/pull/38054#discussion_r1523842432
##########
airflow/models/baseoperator.py:
##########
@@ -635,6 +639,7 @@ class derived from this one results in the creation of a
task object,
runs across execution_dates.
:param max_active_tis_per_dagrun: When set, a task will be able to limit
the concurrent
task instances per DAG run.
+ :param executor: which executor to target when running this task. NOT YET
SUPPORTED
Review Comment:
Nittiest of nitpicks. Some are capitalized, some aren't, feel free to ignore
```suggestion
:param executor: Which executor to target when running this task. NOT
YET SUPPORTED
```
##########
tests/www/views/test_views_tasks.py:
##########
@@ -1089,6 +1089,7 @@ def test_task_instances(admin_client):
"duration": None,
"end_date": None,
"execution_date": DEFAULT_DATE.isoformat(),
Review Comment:
Not expecting you to change it but wow is that test incredibly repetitive.
Maybe after this is merged I'll do a quick PR to move all the fixed values into
a dict and just unpack then in each of these. Would make it much easier to
read and the only values that are called out explicitly would be the ones that
are changing and worth paying specific attention to.
##########
airflow/models/baseoperator.py:
##########
@@ -719,6 +724,8 @@ class derived from this one results in the creation of a
task object,
"on_skipped_callback",
"do_xcom_push",
"multiple_outputs",
+ # TODO: should executor go here? I think so
+ "executor",
Review Comment:
Yeah, that's awkward. I think it goes in there, seems to make sense to
me.
##########
airflow/models/taskinstance.py:
##########
@@ -1252,6 +1254,8 @@ class TaskInstance(Base, LoggingMixin):
queued_dttm = Column(UtcDateTime)
queued_by_job_id = Column(Integer)
pid = Column(Integer)
+ # TODO: Should this be nullable?
+ executor = Column(String(1000))
Review Comment:
I don't think nullable makes sense here, as uranusjr said.. can't think of
a case where "no executor" would be valid?
--
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]
