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 2022/09/12 11:12:38 UTC
[GitHub] [airflow] potiuk opened a new pull request, #26341: Improve 'start-airflow' experience for users and ui developers
potiuk opened a new pull request, #26341:
URL: https://github.com/apache/airflow/pull/26341
Since we moved the "compile-www-assets" to host (and deferred it to pre-commit to prepare and manage appropriate node virtualenv), starting airflow via `start-airflow` command had become slower, adding quite a few seconds always when you run the command. Also it was a bit convoluted to start dev mode - you had to stop and restart webserver and run dev asset recompilation.
This change improves the `start-airflow` experience in several ways:
* the compile-www-assets pre-commit in start airflow runs in a background, daemon thread. This allows start-airflow command to continue even if asset compilation takes quite some time. By the time webserver starts and database initializes and the user launches browser it should be completed anyway.
* the assets are not recompiled if none of the "www" files changed which for most people who run airflow without modifying the UI part will make it really fast to re-run start-airflow
* we have a new flag --skip-assets-compilation added that will skip asset compilation even if the www files were modified
* finally we have a new "--dev-mode" when starting airflow that will run the "dev" mode of assets compilation. And thanks to the background, daemon thread, assets compilation in dev mode will continue running as long as the `start-airflow` command is running
All this should naturally plug in the exisitng start-airflow usage and add new capabilities for those who develop UI (you can run dev mode now without manually restarting anything).
<!--
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 an 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 changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+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 a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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 diff in pull request #26341: Improve 'start-airflow' experience for users and ui developers
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #26341:
URL: https://github.com/apache/airflow/pull/26341#discussion_r968292805
##########
scripts/ci/pre_commit/pre_commit_compile_www_assets.py:
##########
@@ -17,17 +17,31 @@
# under the License.
import os
import subprocess
+import sys
from pathlib import Path
+sys.path.insert(0, str(Path(__file__).parent.resolve())) # make sure common_precommit_utils is imported
+from common_precommit_utils import get_directory_hash # isort: skip # noqa
Review Comment:
I cannot use the nice tool to calculate hash of directories that I use in "doc" (checksumdir), because the scripts are using "node" environment in pre-commit, and we cannot add extra python dependencies, so I have to rely on stdlib. The "get_directory_hash" is rather small and simple.
--
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 diff in pull request #26341: Improve 'start-airflow' experience for users and ui developers
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #26341:
URL: https://github.com/apache/airflow/pull/26341#discussion_r968288245
##########
dev/breeze/src/airflow_breeze/utils/docker_command_utils.py:
##########
@@ -622,23 +621,25 @@ def update_expected_environment_variables(env: Dict[str, str]) -> None:
"BACKEND": "backend",
"COMPOSE_FILE": "compose_files",
"DB_RESET": 'db_reset',
+ "DEV_MODE": 'dev_mode',
Review Comment:
I sorted the list too :)
--
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 merged pull request #26341: Improve 'start-airflow' experience for users and ui developers
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #26341:
URL: https://github.com/apache/airflow/pull/26341
--
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 a diff in pull request #26341: Improve 'start-airflow' experience for users and ui developers
Posted by GitBox <gi...@apache.org>.
josh-fell commented on code in PR #26341:
URL: https://github.com/apache/airflow/pull/26341#discussion_r972177093
##########
scripts/ci/docker-compose/devcontainer.env:
##########
@@ -31,6 +31,7 @@ COMMIT_SHA=
DB_RESET="false"
DEFAULT_BRANCH="main"
DEFAULT_CONSTRAINTS_BRANCH="constraints-main"
+DEF_MODE="true"
Review Comment:
Should this be DEV_MODE?
--
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 #26341: Improve 'start-airflow' experience for users and ui developers
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #26341:
URL: https://github.com/apache/airflow/pull/26341#issuecomment-1243586451
cc: @bbovenzi @pierrejeambrun - apart of improving `start-airflow`, you might find it really useful too as it will allow to enter "dev mode" with start-airflow much more easily `--dev-mode` with launch `yarn dev` in the background as well as -d when starting webserver.
--
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 #26341: Improve 'start-airflow' experience for users and ui developers
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #26341:
URL: https://github.com/apache/airflow/pull/26341#issuecomment-1247223116
(rebased it to include PEP-563)
--
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 diff in pull request #26341: Improve 'start-airflow' experience for users and ui developers
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #26341:
URL: https://github.com/apache/airflow/pull/26341#discussion_r972206685
##########
scripts/ci/docker-compose/devcontainer.env:
##########
@@ -31,6 +31,7 @@ COMMIT_SHA=
DB_RESET="false"
DEFAULT_BRANCH="main"
DEFAULT_CONSTRAINTS_BRANCH="constraints-main"
+DEF_MODE="true"
Review Comment:
```suggestion
DEV_MODE="true"
```
--
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] jedcunningham commented on a diff in pull request #26341: Improve 'start-airflow' experience for users and ui developers
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #26341:
URL: https://github.com/apache/airflow/pull/26341#discussion_r971373620
##########
dev/breeze/src/airflow_breeze/commands/developer_commands.py:
##########
@@ -213,6 +213,18 @@ def shell(
@option_mount_sources
@option_integration
@option_image_tag_for_running
+@click.option(
+ '--skip-asset-compilation',
+ help="Skips compilation of assets when starting airflow even if the content of www changed "
+ "(mutually exclusive with --dev-mode).",
+ is_flag=True,
+)
+@click.option(
+ '--dev-mode',
+ help="Starts webserver in dev mode (assets are always recompiled in this case when starting "
Review Comment:
```suggestion
help="Starts webserver in dev mode (assets are always recompiled in this case when starting) "
```
Oh no, you dropped one!
##########
dev/breeze/src/airflow_breeze/commands/developer_commands.py:
##########
@@ -236,15 +248,24 @@ def start_airflow(
use_packages_from_dist: bool,
package_format: str,
force_build: bool,
+ skip_asset_compilation: bool,
+ dev_mode: bool,
image_tag: str | None,
db_reset: bool,
answer: str | None,
platform: str | None,
extra_args: tuple,
):
- """Enter breeze environment and starts all Airflow components in the tmux session."""
- if use_airflow_version is None:
- run_compile_www_assets(dev=False, verbose=verbose, dry_run=dry_run)
+ """
+ Enter breeze environment and starts all Airflow components in the tmux session.
+ Compile assets if contents of www directory changed."""
Review Comment:
```suggestion
Compile assets if contents of www directory changed.
"""
```
nit
--
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 #26341: Improve 'start-airflow' experience for users and ui developers
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #26341:
URL: https://github.com/apache/airflow/pull/26341#issuecomment-1247222862
Looking forward to merging this one :). Should be good improvement for many users.
--
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 diff in pull request #26341: Improve 'start-airflow' experience for users and ui developers
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #26341:
URL: https://github.com/apache/airflow/pull/26341#discussion_r972206296
##########
scripts/ci/docker-compose/devcontainer.env:
##########
@@ -31,6 +31,7 @@ COMMIT_SHA=
DB_RESET="false"
DEFAULT_BRANCH="main"
DEFAULT_CONSTRAINTS_BRANCH="constraints-main"
+DEF_MODE="true"
Review Comment:
OF COURSE :)
--
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