Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a calendar field to choose the execution date of the DAG when triggering it #16141

Merged
merged 6 commits into from
Jul 8, 2021

Conversation

JPonte
Copy link
Contributor

@JPonte JPonte commented May 28, 2021

Currently the Trigger DAG always triggers a DAG run with now() as execution date. Since execution_date is a parameter of the dagrun the same way conf is, it makes sense to be able to change it in the UI as well.
image

@boring-cyborg boring-cyborg bot added area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels May 28, 2021
Copy link
Contributor

@bbovenzi bbovenzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea! But it looks like test_views_trigger_dag.py is failing

@JPonte
Copy link
Contributor Author

JPonte commented May 29, 2021

@bbovenzi I fixed the failing test but the CI still fails for mssql but it doesn't really say where and I can't even seem to run it on breeze.

airflow/www/views.py Outdated Show resolved Hide resolved
airflow/www/views.py Outdated Show resolved Hide resolved
@JPonte
Copy link
Contributor Author

JPonte commented May 30, 2021

Thanks @mrbaguvix

Copy link
Contributor

@bbovenzi bbovenzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! Past and future dates work great.

@bbovenzi
Copy link
Contributor

@ashb or @ryanahamilton Can we restart the CI here?

@ashb ashb closed this Jun 11, 2021
@ashb ashb reopened this Jun 11, 2021
@ashb
Copy link
Member

ashb commented Jun 11, 2021

Restarted.

@ashb
Copy link
Member

ashb commented Jun 11, 2021

    ERROR: HTTP error 404 while getting https://github.com/apache/airflow/archive/master.tar.gz#egg=apache-airflow[devel_ci]

This PR needs rebasing on to get the master->main rename

@bbovenzi
Copy link
Contributor

@JPonte could you resolve the conflicts and hopefully we can get this merged soon.

@JPonte
Copy link
Contributor Author

JPonte commented Jun 22, 2021

@bbovenzi I fixed the conflicts with main but the checks still timeout on the "Wait for CI images" job.

@ashb ashb added this to the Airflow 2.2 milestone Jun 22, 2021
Copy link
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you expand the tests to see what the behaviour is when an invalid date is submitted please?

Something like: it should still be a 200, but there should be a error message somewhere.

airflow/www/decorators.py Outdated Show resolved Hide resolved
airflow/www/decorators.py Outdated Show resolved Hide resolved
Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@github-actions
Copy link

github-actions bot commented Jul 8, 2021

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.

@github-actions github-actions bot added the okay to merge It's ok to merge this PR as it does not require more tests label Jul 8, 2021
@ryanahamilton ryanahamilton merged commit db6acd9 into apache:main Jul 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues okay to merge It's ok to merge this PR as it does not require more tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants