You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/04/09 15:47:22 UTC
[GitHub] [airflow] marcusianlevine opened a new pull request #15308: Fix used_group_ids in partial_subset (#13700)
marcusianlevine opened a new pull request #15308:
URL: https://github.com/apache/airflow/pull/15308
closes: #13700
This should address the root cause by removing any task_ids that are not in the subset DAG from the `used_group_ids` - might not be the most efficient way to do this since we're regenerating the entire set
--
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:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #15308: Fix used_group_ids in partial_subset (#13700)
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #15308:
URL: https://github.com/apache/airflow/pull/15308#issuecomment-823491892
Thanks @marcusianlevine
--
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:
users@infra.apache.org
[GitHub] [airflow] marcusianlevine commented on pull request #15308: Fix used_group_ids in partial_subset (#13700)
Posted by GitBox <gi...@apache.org>.
marcusianlevine commented on pull request #15308:
URL: https://github.com/apache/airflow/pull/15308#issuecomment-816932508
Thanks Kaxil, I should have opened as a draft - I'll sort out the broken test
--
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:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #15308: Fix used_group_ids in partial_subset (#13700)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #15308:
URL: https://github.com/apache/airflow/pull/15308#issuecomment-823342293
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
--
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:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #15308: Fix used_group_ids in partial_subset (#13700)
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #15308:
URL: https://github.com/apache/airflow/pull/15308#issuecomment-816914688
The tests are failing unfortunately, can you take a look please:
https://github.com/apache/airflow/pull/15308/checks?check_run_id=2307747778#step:6:14417
--
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:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #15308: Fix used_group_ids in partial_subset (#13700)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #15308:
URL: https://github.com/apache/airflow/pull/15308#issuecomment-823476204
[The Workflow run](https://github.com/apache/airflow/actions/runs/767597919) is cancelling this PR. Building images for the PR has failed. Follow the workflow link to check the reason.
--
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:
users@infra.apache.org
[GitHub] [airflow] kaxil merged pull request #15308: Fix used_group_ids in partial_subset (#13700)
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #15308:
URL: https://github.com/apache/airflow/pull/15308
--
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:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #15308: Fix used_group_ids in partial_subset (#13700)
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #15308:
URL: https://github.com/apache/airflow/pull/15308#discussion_r614465134
##########
File path: airflow/models/dag.py
##########
@@ -1504,6 +1504,8 @@ def filter_task_group(group, parent_group):
if isinstance(child, BaseOperator):
if child.task_id in dag.task_dict:
copied.children[child.task_id] = dag.task_dict[child.task_id]
+ else:
+ copied.used_group_ids.discard(child.task_id)
Review comment:
Can you write a unit test for it please?
--
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:
users@infra.apache.org
[GitHub] [airflow] marcusianlevine commented on a change in pull request #15308: Fix used_group_ids in partial_subset (#13700)
Posted by GitBox <gi...@apache.org>.
marcusianlevine commented on a change in pull request #15308:
URL: https://github.com/apache/airflow/pull/15308#discussion_r616027987
##########
File path: airflow/models/dag.py
##########
@@ -1504,6 +1504,8 @@ def filter_task_group(group, parent_group):
if isinstance(child, BaseOperator):
if child.task_id in dag.task_dict:
copied.children[child.task_id] = dag.task_dict[child.task_id]
+ else:
+ copied.used_group_ids.discard(child.task_id)
Review comment:
I added another assert to an existing test of the `sub_dag` function, but the quarantined tests are failing (not sure if that's a problem)
--
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:
users@infra.apache.org
[GitHub] [airflow] marcusianlevine commented on pull request #15308: Fix used_group_ids in partial_subset (#13700)
Posted by GitBox <gi...@apache.org>.
marcusianlevine commented on pull request #15308:
URL: https://github.com/apache/airflow/pull/15308#issuecomment-819100126
@kaxil I got the test passing by switching from `.remove` to `.discard`
--
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:
users@infra.apache.org