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/09/23 11:31:25 UTC
[GitHub] [airflow] ashb opened a new pull request #18462: Don't use flash for "same-page" UI messages.
ashb opened a new pull request #18462:
URL: https://github.com/apache/airflow/pull/18462
The flash is designed for setting a message on one page and then showing
it after a redirect, so in the case of these UI warnings they were being
shown twice due to an early redirect.
I could have chosen to fix this by moving the checks to after the
`reset_tags` redirect, but it _also_ felt wrong to me to have HTML in
the view, so I have chosen to move it in to the template where it
belongs.
To (marginally) reduce boilerplate I have created `message()` "macro"
(Jinja macro, a.k.a. function; not to be confused with what Airflow
templates call a macro, but is in fact just a template global) that
handles the formatting for messages.
Closes https://github.com/apache/airflow/issues/17727
--
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] ashb merged pull request #18462: Don't use flash for "same-page" UI messages.
Posted by GitBox <gi...@apache.org>.
ashb merged pull request #18462:
URL: https://github.com/apache/airflow/pull/18462
--
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 #18462: Don't use flash for "same-page" UI messages.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #18462:
URL: https://github.com/apache/airflow/pull/18462#issuecomment-925742374
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] ashb commented on a change in pull request #18462: Don't use flash for "same-page" UI messages.
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #18462:
URL: https://github.com/apache/airflow/pull/18462#discussion_r714747542
##########
File path: airflow/www/views.py
##########
@@ -541,35 +546,6 @@ def health(self):
)
def index(self):
"""Home view."""
- unit_test_mode: bool = conf.getboolean('core', 'UNIT_TEST_MODE')
-
- if not unit_test_mode and "sqlite" in conf.get("core", "sql_alchemy_conn"):
- db_doc_page = get_docs_url("howto/set-up-database.html")
- flash(
- Markup(
- "Usage of <b>SQLite</b> detected. It should only be used for dev/testing. "
- "Do not use <b>SQLite</b> as metadata DB in production. "
- "We recommend using Postgres or MySQL. "
- f"<a href='{db_doc_page}'><b>Click here</b></a> for more information."
- ),
- category="warning",
- )
-
- if not unit_test_mode and conf.get("core", "executor") == "SequentialExecutor":
- exec_doc_page = get_docs_url("executor/index.html")
- flash(
- Markup(
- "Usage of <b>SequentialExecutor</b> detected. "
- "Do not use <b>SequentialExecutor</b> in production. "
- f"<a href='{exec_doc_page}'><b>Click here</b></a> for more information."
- ),
- category="warning",
- )
Review comment:
Content is moved to the templates, and the checks are moved to class args (so they aren't recomputed on every page request)
--
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 #18462: Don't use flash for "same-page" UI messages.
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #18462:
URL: https://github.com/apache/airflow/pull/18462#discussion_r714759883
##########
File path: airflow/www/views.py
##########
@@ -541,35 +546,6 @@ def health(self):
)
def index(self):
"""Home view."""
- unit_test_mode: bool = conf.getboolean('core', 'UNIT_TEST_MODE')
-
- if not unit_test_mode and "sqlite" in conf.get("core", "sql_alchemy_conn"):
- db_doc_page = get_docs_url("howto/set-up-database.html")
- flash(
- Markup(
- "Usage of <b>SQLite</b> detected. It should only be used for dev/testing. "
- "Do not use <b>SQLite</b> as metadata DB in production. "
- "We recommend using Postgres or MySQL. "
- f"<a href='{db_doc_page}'><b>Click here</b></a> for more information."
- ),
- category="warning",
- )
-
- if not unit_test_mode and conf.get("core", "executor") == "SequentialExecutor":
- exec_doc_page = get_docs_url("executor/index.html")
- flash(
- Markup(
- "Usage of <b>SequentialExecutor</b> detected. "
- "Do not use <b>SequentialExecutor</b> in production. "
- f"<a href='{exec_doc_page}'><b>Click here</b></a> for more information."
- ),
- category="warning",
- )
Review comment:
Ah I somehow missed the part in `dag.html`. Makes sense now.
--
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 #18462: Don't use flash for "same-page" UI messages.
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #18462:
URL: https://github.com/apache/airflow/pull/18462#discussion_r714727722
##########
File path: airflow/www/views.py
##########
@@ -541,35 +546,6 @@ def health(self):
)
def index(self):
"""Home view."""
- unit_test_mode: bool = conf.getboolean('core', 'UNIT_TEST_MODE')
-
- if not unit_test_mode and "sqlite" in conf.get("core", "sql_alchemy_conn"):
- db_doc_page = get_docs_url("howto/set-up-database.html")
- flash(
- Markup(
- "Usage of <b>SQLite</b> detected. It should only be used for dev/testing. "
- "Do not use <b>SQLite</b> as metadata DB in production. "
- "We recommend using Postgres or MySQL. "
- f"<a href='{db_doc_page}'><b>Click here</b></a> for more information."
- ),
- category="warning",
- )
-
- if not unit_test_mode and conf.get("core", "executor") == "SequentialExecutor":
- exec_doc_page = get_docs_url("executor/index.html")
- flash(
- Markup(
- "Usage of <b>SequentialExecutor</b> detected. "
- "Do not use <b>SequentialExecutor</b> in production. "
- f"<a href='{exec_doc_page}'><b>Click here</b></a> for more information."
- ),
- category="warning",
- )
Review comment:
Are these two flashes removed without replacement?
--
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