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/07/05 13:07:28 UTC
[GitHub] [airflow] potiuk opened a new pull request #16815: Fixed DAG triggering with parameters
potiuk opened a new pull request #16815:
URL: https://github.com/apache/airflow/pull/16815
There was a bad cherry-pick when merging #15057 that has not
been caught because of quarantined test.
Fixes: #16810
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ 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] uranusjr commented on a change in pull request #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#discussion_r663919211
##########
File path: tests/www/views/test_views_trigger_dag.py
##########
@@ -52,7 +52,6 @@ def test_trigger_dag_button(admin_client):
assert run.run_type == DagRunType.MANUAL
-@pytest.mark.quarantined
Review comment:
Should we remove this in main 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] kaxil commented on pull request #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#issuecomment-874132231
Yeah must be a side-effect, as the test passes for me locally
--
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 a change in pull request #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#discussion_r663932479
##########
File path: tests/www/views/test_views_trigger_dag.py
##########
@@ -52,7 +52,6 @@ def test_trigger_dag_button(admin_client):
assert run.run_type == DagRunType.MANUAL
-@pytest.mark.quarantined
Review comment:
Yep. Looking at the status of it . I have disabled the automated status gathering of Flaky tests some time ago as it was not as useful due to timeouts - but we should likely come back to this.
--
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 #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#issuecomment-874100771
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main 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] kaxil commented on a change in pull request #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#discussion_r663920722
##########
File path: tests/www/views/test_views_trigger_dag.py
##########
@@ -52,7 +52,6 @@ def test_trigger_dag_button(admin_client):
assert run.run_type == DagRunType.MANUAL
-@pytest.mark.quarantined
Review comment:
This test was flaky for some time, we should probably take a look on how it was doing recently
--
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 closed pull request #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
potiuk closed pull request #16815:
URL: https://github.com/apache/airflow/pull/16815
--
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 a change in pull request #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#discussion_r663919425
##########
File path: tests/www/views/test_views_trigger_dag.py
##########
@@ -52,7 +52,6 @@ def test_trigger_dag_button(admin_client):
assert run.run_type == DagRunType.MANUAL
-@pytest.mark.quarantined
Review comment:
Absolutely
##########
File path: tests/www/views/test_views_trigger_dag.py
##########
@@ -52,7 +52,6 @@ def test_trigger_dag_button(admin_client):
assert run.run_type == DagRunType.MANUAL
-@pytest.mark.quarantined
Review comment:
I can create a PR to main with de-quarantining and we can cherry-pick both PRs instead.
##########
File path: tests/www/views/test_views_trigger_dag.py
##########
@@ -52,7 +52,6 @@ def test_trigger_dag_button(admin_client):
assert run.run_type == DagRunType.MANUAL
-@pytest.mark.quarantined
Review comment:
Yep. Looking at the status of it . I have disabled the automated status gathering of Flaky tests some time ago as it was not as useful due to timeouts - but we should likely come back to this.
--
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 commented on pull request #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#issuecomment-874100085
Looks like we already had a fix: https://github.com/apache/airflow/pull/16648 in main .
--
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 commented on a change in pull request #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#discussion_r663920722
##########
File path: tests/www/views/test_views_trigger_dag.py
##########
@@ -52,7 +52,6 @@ def test_trigger_dag_button(admin_client):
assert run.run_type == DagRunType.MANUAL
-@pytest.mark.quarantined
Review comment:
This test was flaky for some time, we should probably take a look on how it was doing recently
--
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 a change in pull request #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#discussion_r663919425
##########
File path: tests/www/views/test_views_trigger_dag.py
##########
@@ -52,7 +52,6 @@ def test_trigger_dag_button(admin_client):
assert run.run_type == DagRunType.MANUAL
-@pytest.mark.quarantined
Review comment:
Absolutely
--
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 #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#issuecomment-874100771
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main 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] potiuk commented on pull request #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#issuecomment-874113183
I think we need to come back to the idea of keeping status of quarantined tests in issues (and some way of nagging the committers to fix them).
I have high hopes for the new GitHub issues (Private Beta) to be able to it much in much simpler and more manageable way (working with GitHub PM for GitHub issues to enable it for `apache` organisation - hopefully next week).
The new GitHub issues have better ways to organise them, split into sub-issues and better API for automation of any tasks around those (apparently). https://github.com/features/issues/
--
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 commented on pull request #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#issuecomment-874100085
--
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 #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#issuecomment-874134253
Feb 6 2020: f0c31c597a0d55e4602ca94725704e1cd4a066e3
OK: Seems the test was Never expected to work. It was added as "xfail" from the beginning :)
+ @pytest.mark.xfail(condition=True, reason="This test might be flaky on mysql")
+ def test_trigger_dag_conf(self):
--
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 a change in pull request #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#discussion_r663919933
##########
File path: tests/www/views/test_views_trigger_dag.py
##########
@@ -52,7 +52,6 @@ def test_trigger_dag_button(admin_client):
assert run.run_type == DagRunType.MANUAL
-@pytest.mark.quarantined
Review comment:
OK will send a PR.
--
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 #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#issuecomment-874108048
--
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 commented on pull request #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#issuecomment-874118062
Meanwhile I have cherry-picked the original commit to both v2-1-stable and v2-1-test
And separately created https://github.com/apache/airflow/pull/16819 to require approvals for mering to v2-1-stable
--
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 closed pull request #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
potiuk closed pull request #16815:
URL: https://github.com/apache/airflow/pull/16815
--
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 a change in pull request #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#discussion_r663919211
##########
File path: tests/www/views/test_views_trigger_dag.py
##########
@@ -52,7 +52,6 @@ def test_trigger_dag_button(admin_client):
assert run.run_type == DagRunType.MANUAL
-@pytest.mark.quarantined
Review comment:
Should we remove this in main as well?
##########
File path: tests/www/views/test_views_trigger_dag.py
##########
@@ -52,7 +52,6 @@ def test_trigger_dag_button(admin_client):
assert run.run_type == DagRunType.MANUAL
-@pytest.mark.quarantined
Review comment:
OK will send a PR.
--
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 a change in pull request #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#discussion_r663920163
##########
File path: tests/www/views/test_views_trigger_dag.py
##########
@@ -52,7 +52,6 @@ def test_trigger_dag_button(admin_client):
assert run.run_type == DagRunType.MANUAL
-@pytest.mark.quarantined
Review comment:
I can create a PR to main with de-quarantining and we can cherry-pick both PRs 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] potiuk commented on pull request #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#issuecomment-874649929
Cosing this as already cherry-picked by @kaxil. Looking at quarantine removal in #16818
--
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 #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#issuecomment-874130113
The test is apparently failing in main even AFTER the #16648 fix in main - so I think it was not flaky - it was marked as quarantined because it just did not work :). Surprisingly it worked for me when I fixed it in v2_1_stable PR. Let's see how this one will do - maybe it is a side-effect of some other tests.
The refactors on test_views do not make it easy to track the origin (I cannot see an issue created for that - I try to always create an issue when marking something as quarantined) but I tracked down when it was marked quarantined - Oct 11 2020
5bc5994c2c1a8f73a644e29e98d3d132c6097ab2
```
- @pytest.mark.xfail(condition=using_mysql, reason="This test might be flaky on mysql")
+ @pytest.mark.quarantined
def test_trigger_dag_conf(self):
```
Now. Let's see if I can track how long time ago we expected it to fail :)
--
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 #16815: Fixed DAG triggering with parameters
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #16815:
URL: https://github.com/apache/airflow/pull/16815#issuecomment-874108048
Ok. Let me merge this one and we can see how the de-quarantining PR is doing.
--
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