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/08/06 03:44:32 UTC
[GitHub] [airflow] josh-fell opened a new pull request #17456: Adding TaskGroup support in chain()
josh-fell opened a new pull request #17456:
URL: https://github.com/apache/airflow/pull/17456
Related to: #17083, #16635
This PR ensures that `TaskGroups` can be used to set dependencies while calling the `chain()` method. Support for `XComArgs` and `EdgeModifiers` has been implemented in previous PRs: #16732, #17099
---
**^ 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 change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+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 [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
--
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] potiuk commented on pull request #17456: Adding TaskGroup support in chain()
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #17456:
URL: https://github.com/apache/airflow/pull/17456#issuecomment-900219115
> Those two (`test_mark_success_on_success_callback` and `test_verify_integrity_if_dag_not_changed`) have been flaky recently; they are likely not related to your changes.
However any help to make them non-flaky is definitely most welcome :)
--
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] josh-fell commented on pull request #17456: Adding TaskGroup support in chain()
Posted by GitBox <gi...@apache.org>.
josh-fell commented on pull request #17456:
URL: https://github.com/apache/airflow/pull/17456#issuecomment-899937698
FYI - the two failing tests pass locally for me. Let me know if there is anything I can do to help these tests pass here.
**MSSQL 2017-latest, Python 3.7**
**MSSQL 2019-latest, Python 3.9**
--
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 #17456: Adding TaskGroup support in chain()
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #17456:
URL: https://github.com/apache/airflow/pull/17456#issuecomment-900015553
Those two (`test_mark_success_on_success_callback` and `test_verify_integrity_if_dag_not_changed`) have been flaky recently; they are likely not related to your changes.
--
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] kaxil merged pull request #17456: Adding TaskGroup support in chain()
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #17456:
URL: https://github.com/apache/airflow/pull/17456
--
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] github-actions[bot] commented on pull request #17456: Adding TaskGroup support in chain()
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17456:
URL: https://github.com/apache/airflow/pull/17456#issuecomment-894005427
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 main 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] josh-fell edited a comment on pull request #17456: Adding TaskGroup support in chain()
Posted by GitBox <gi...@apache.org>.
josh-fell edited a comment on pull request #17456:
URL: https://github.com/apache/airflow/pull/17456#issuecomment-901537912
> > Those two (`test_mark_success_on_success_callback` and `test_verify_integrity_if_dag_not_changed`) have been flaky recently; they are likely not related to your changes.
>
> However any help to make them non-flaky is definitely most welcome :)
Happy to take a look and help any way I can. Perhaps a silly question but is this test always failing for the same exception? If not, is there a set of logs I can look through to see any failure trends?
```shell
> assert success_callback_called.value == 1
E assert 0 == 1
E + where 0 = <Synchronized wrapper for c_int(0)>.value
```
--
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] josh-fell commented on pull request #17456: Adding TaskGroup support in chain()
Posted by GitBox <gi...@apache.org>.
josh-fell commented on pull request #17456:
URL: https://github.com/apache/airflow/pull/17456#issuecomment-901537912
> > Those two (`test_mark_success_on_success_callback` and `test_verify_integrity_if_dag_not_changed`) have been flaky recently; they are likely not related to your changes.
>
> However any help to make them non-flaky is definitely most welcome :)
Happy to take a look and help any way I can. Is this test always failing for the same exception? If not, is there a set of logs I can look through to see any failure trends?
```shell
> assert success_callback_called.value == 1
E assert 0 == 1
E + where 0 = <Synchronized wrapper for c_int(0)>.value
```
--
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] potiuk commented on pull request #17456: Adding TaskGroup support in chain()
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #17456:
URL: https://github.com/apache/airflow/pull/17456#issuecomment-901540998
> Happy to take a look and help any way I can. Is this test always failing for the same exception? If not, is there a set of logs I can look through to see any failure trends?
Only by looking at individual failed actions. I used to have a "quarantine" summary of tasks, but it has proven to be really a pain to maintain.
But that actually be a good idea, to keep such status of all failed tests somewhere. Maybe you could do that :). I used to keep quarantined tests status in GitHub Issue (as strange as it might seem) but that was a bit "clunky". Maybe we could keep it somewhere else. Google Spreadsheet might be a possibility for one.
We could push status of failed tests for every 'master' merge push. to make it more "robust" and only include stuff that should "work".
--
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