You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "bbovenzi (via GitHub)" <gi...@apache.org> on 2023/08/04 08:09:31 UTC
[GitHub] [airflow] bbovenzi opened a new pull request, #33103: Use task_id as back for node if no label
bbovenzi opened a new pull request, #33103:
URL: https://github.com/apache/airflow/pull/33103
Fixes: #33093
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #33103: Use task_id as backup if no task.label
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on PR #33103:
URL: https://github.com/apache/airflow/pull/33103#issuecomment-1665196356
Where does `label` come from? Maybe we should fix that at the Python side instead.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dstandish commented on pull request #33103: Use task_id as backup if no task.label
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on PR #33103:
URL: https://github.com/apache/airflow/pull/33103#issuecomment-1670555673
I was about to say that I think we can close this because after https://github.com/apache/airflow/pull/33102, setting deps in a context no longer "auto adds" tasks to the context. So now the sample dag posted in https://github.com/apache/airflow/issues/33093 will produce this graph:
<img width="690" alt="image" src="https://github.com/apache/airflow/assets/15932138/e350cd60-89c6-4404-9146-6846ace94a74">
I.e. nothing will be in the task group.
BUT...
then i added the tasks to the context with `add_task`:
```python
with TaskGroup("my_group") as tg:
setup1 >> work1 >> teardown1
setup1 >> teardown1
tg.add_task(setup1)
tg.add_task(teardown1)
tg.add_task(work1)
tg >> work2
```
And now we encounter the issue again:
<img width="936" alt="image" src="https://github.com/apache/airflow/assets/15932138/087a51d5-cbc3-4832-aa97-4b8240522566">
I'll poke around and see if I can figure anything out.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy closed pull request #33103: Use task_id as backup if no task.label
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy closed pull request #33103: Use task_id as backup if no task.label
URL: https://github.com/apache/airflow/pull/33103
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dstandish commented on pull request #33103: Use task_id as backup if no task.label
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on PR #33103:
URL: https://github.com/apache/airflow/pull/33103#issuecomment-1670561451
Ok so the problem is this logic:
```python
@property
def label(self) -> str | None:
tg = self.task_group
if tg and tg.node_id and tg.prefix_group_id:
# "task_group_id.task_id" -> "task_id"
return self.node_id[len(tg.node_id) + 1 :]
return self.node_id
```
The issue is when the task is instantiated outside of the group, it doesn't get prefixed. So `self.node_id[len(tg.node_id) + 1 :]` is `''`
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on pull request #33103: Use task_id as backup if no task.label
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy commented on PR #33103:
URL: https://github.com/apache/airflow/pull/33103#issuecomment-1670772230
> Removing `add_task` sounds reasonable, I think that’s proposed once previously for another reason as well?
Yeah. We should remove it. We previously wanted to make it `add_tasks`
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dstandish commented on pull request #33103: Use task_id as backup if no task.label
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on PR #33103:
URL: https://github.com/apache/airflow/pull/33103#issuecomment-1670565064
An odd consequence of this is if the task group id is short, we'll actually get a partial name:
<img width="945" alt="image" src="https://github.com/apache/airflow/assets/15932138/8e85b069-0a74-492d-ab2d-8ffe0fddb36d">
code to repro:
```python
with DAG(dag_id="leaves_ignored", start_date=pendulum.now()):
@task
def setup1():
...
@task
def setup2():
...
@task
def teardown1():
...
@task
def teardown2():
...
@task
def work1():
...
@task
def work2():
...
setup1 = setup1()
# setup2 = setup2()
teardown1 = teardown1()
# teardown2 = teardown2()
work1 = work1()
work2 = work2()
with TaskGroup("tg") as tg:
setup1 >> work1 >> teardown1
setup1 >> teardown1
tg.add_task(setup1)
tg.add_task(teardown1)
tg.add_task(work1)
tg >> work2
```
Maybe we should remove `add_task` for task group for now, wdyt @ephraimbuddy @uranusjr ?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #33103: Use task_id as backup if no task.label
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on PR #33103:
URL: https://github.com/apache/airflow/pull/33103#issuecomment-1670753247
Removing `add_task` sounds reasonable, I think that’s proposed once previously as well?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bbovenzi commented on pull request #33103: Use task_id as backup if no task.label
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on PR #33103:
URL: https://github.com/apache/airflow/pull/33103#issuecomment-1669543413
@uranusjr I agree. I just don't know why label isn't being populated. @dstandish With your example dag, do you have any ideas?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org