Skip to content

Commit

Permalink
Move javascript compilation to host (#25169)
Browse files Browse the repository at this point in the history
Previously, in order to keep consistent development environment
we've compiled javascript in the CI image. However we can utilise
power of pre-commmit for setting the node environment for all
contributors automatically. Instead of compiling the javascript
in the image, we can compile it via pre-commit in the host.

This can be done thanks to the new python breeze which is far more
flexible and can now add execution of compilation of the javascript when
needed and using pre-commit environments.

Thanks to that, we can vastly simplify the Dockerfiles and scripts that
are used to automatically build or signal that the assets need
recompilation. We can basically assume that the assets were prepared
outside of the image building (and breeze makes sure it happens)

The changes:

* node.js is not needed in images (neither PROD build nor CI)
* no need for multiple asset compilation scripts. All is done
  via pre-commit environment with `breeze compile-www-assets``
  command
* lint checks for UI do not need the docker image any more
  (they are also based on pre-commit environment)
* no more checks/warnings when you enter the image
* start-airflow command builds the compilation before entering
* prepare-airflow-package runs asset compilation before entering
  docker airflow building.

(cherry picked from commit acff129)
  • Loading branch information
potiuk committed Jul 21, 2022
1 parent f18e165 commit 60bc1ce
Show file tree
Hide file tree
Showing 38 changed files with 693 additions and 979 deletions.
9 changes: 3 additions & 6 deletions .dockerignore
Original file line number Diff line number Diff line change
Expand Up @@ -74,18 +74,15 @@
!setup.cfg
!setup.py
!manifests
!generated
# Now - ignore unnecessary files inside allowed directories
# This goes after the allowed directories

# Git version is dynamically generated
airflow/git_version

# Exclude static www files generated by NPM
airflow/www/static/coverage
airflow/www/static/dist
# Exclude mode_modules pulled by "yarn" for compilation of www files generated by NPM
airflow/www/node_modules
# Exclude static ui files generated by NPM
airflow/ui/build
airflow/ui/node_modules

# Exclude link to docs
Expand All @@ -99,7 +96,7 @@ airflow/www/static/docs
**/env/
**/build/
**/develop-eggs/
**/dist/
/dist/
**/downloads/
**/eggs/
**/.eggs/
Expand Down
1 change: 0 additions & 1 deletion .github/boring-cyborg.yml
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ labelPRBasedOnFilePath:
- airflow/www/.eslintrc
- airflow/www/.stylelintignore
- airflow/www/.stylelintrc
- airflow/www/compile_assets.sh
- airflow/www/package.json
- airflow/www/webpack.config.js
- airflow/www/yarn.lock
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/build-images.yml
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,8 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
breeze static-checks --type update-providers-dependencies --all-files
--show-diff-on-failure --color always || true
if: needs.build-info.outputs.default-branch == 'main'
- name: Compile www assets
run: breeze compile-www-assets
- name: >-
Build & Push AMD64 CI images ${{ env.IMAGE_TAG_FOR_THE_BUILD }}
${{ needs.build-info.outputs.all-python-versions-list-as-string }}
Expand Down Expand Up @@ -343,6 +345,8 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
run: breeze prepare-airflow-package --package-format wheel --version-suffix-for-pypi dev0
- name: "Move dist packages to docker-context files"
run: mv -v ./dist/*.whl ./docker-context-files
- name: Compile www assets
run: breeze compile-www-assets
- name: >-
Build & Push PROD images ${{ env.IMAGE_TAG_FOR_THE_BUILD }}
${{ needs.build-info.outputs.all-python-versions-list-as-string }}
Expand Down
8 changes: 8 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,11 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
if: >
needs.build-info.outputs.in-workflow-build == 'true' &&
needs.build-info.outputs.default-branch == 'main'
- name: Compile www assets
run: breeze compile-www-assets
if: >
needs.build-info.outputs.in-workflow-build == 'true' &&
needs.build-info.outputs.default-branch == 'main'
- name: >
Build & Push CI images ${{ env.IMAGE_TAG_FOR_THE_BUILD }}
${{ needs.build-info.outputs.all-python-versions-list-as-string }}
Expand Down Expand Up @@ -448,6 +453,9 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
- name: "Move dist packages to docker-context files"
run: mv -v ./dist/*.whl ./docker-context-files
if: needs.build-info.outputs.in-workflow-build == 'true'
- name: Compile www assets
run: breeze compile-www-assets
if: needs.build-info.outputs.in-workflow-build == 'true'
- name: >
Build & Push PROD images ${{ env.IMAGE_TAG_FOR_THE_BUILD }}
${{ needs.build-info.outputs.all-python-versions-list-as-string }}
Expand Down
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ unittests.db
# Airflow temporary artifacts
airflow/git_version
airflow/www/static/coverage/
airflow/www/static/dist
airflow/www/*.log

/logs/
airflow-webserver.pid
Expand Down
65 changes: 48 additions & 17 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ repos:
exclude: ^airflow/_vendor/|^RELEASE_NOTES\.txt$|^airflow/www/static/css/material-icons\.css$|^images/.*$
args:
- --ignore-words=docs/spelling_wordlist.txt
- --skip=docs/*/commits.rst,airflow/providers/*/*.rst,*.lock,INTHEWILD.md,*.min.js,docs/apache-airflow/pipeline_example.csv
- --skip=docs/*/commits.rst,airflow/providers/*/*.rst,*.lock,INTHEWILD.md,*.min.js,docs/apache-airflow/pipeline_example.csv,airflow/www/*.log
- --exclude-file=.codespellignorelines
- repo: local
hooks:
Expand Down Expand Up @@ -582,9 +582,20 @@ repos:
name: stylelint
entry: "stylelint"
language: node
language_version: 18.6.0
files: ^airflow/www/.*\.(css|scss|sass)$
# Keep dependency versions in sync w/ airflow/www/package.json
additional_dependencies: ['stylelint@13.3.1', 'stylelint-config-standard@20.0.0']
- id: compile-www-assets
name: Compile www assets
language: node
language_version: 18.6.0
stages: ['manual']
'types_or': [javascript, tsx, ts]
files: ^airflow/www/
entry: ./scripts/ci/pre_commit/pre_commit_compile_www_assets.py
pass_filenames: false
additional_dependencies: ['yarn@1.22.19']
- id: check-providers-init-file-missing
name: Provider init file is missing
pass_filenames: false
Expand Down Expand Up @@ -657,6 +668,7 @@ repos:
description: Checks the style of Markdown files.
entry: markdownlint
language: node
language_version: 18.6.0
types: [markdown]
files: \.(md|mdown|markdown)$
additional_dependencies: ['markdownlint-cli']
Expand Down Expand Up @@ -777,6 +789,41 @@ repos:
files: newsfragments/.*\.rst
entry: ./scripts/ci/pre_commit/pre_commit_newsfragments.py
pass_filenames: true
# We sometimes won't have newsfragments in the repo, so always run it so `check-hooks-apply` passes
# This is fast, so not too much downside
always_run: true
- id: check-system-tests-tocs
name: Check that system tests is properly added
entry: ./scripts/ci/pre_commit/pre_commit_check_system_tests_hidden_in_index.py
language: python
pass_filenames: true
files: ^docs/apache-airflow-providers-[^/]*/index\.rst$
additional_dependencies: ['rich>=12.4.4', 'pyyaml']
- id: create-missing-init-py-files-tests
name: Create missing init.py files in tests
entry: ./scripts/ci/pre_commit/pre_commit_check_init_in_tests.py
language: python
additional_dependencies: ['rich>=12.4.4']
pass_filenames: false
files: ^tests/.*\.py$
- id: lint-javascript
name: ESLint against airflow/ui
language: node
language_version: 18.6.0
'types_or': [javascript, tsx, ts]
files: ^airflow/ui/
entry: ./scripts/ci/pre_commit/pre_commit_ui_lint.py
pass_filenames: false
additional_dependencies: ['yarn@1.22.19']
- id: lint-javascript
name: ESLint against current UI JavaScript files
language: node
language_version: 18.6.0
'types_or': [javascript, tsx, ts]
files: ^airflow/www/static/js/
entry: ./scripts/ci/pre_commit/pre_commit_www_lint.py
additional_dependencies: ['yarn@1.22.19']
pass_filenames: false
## ADD MOST PRE-COMMITS ABOVE THAT LINE
# The below pre-commits are those requiring CI image to be built
- id: run-mypy
Expand Down Expand Up @@ -817,22 +864,6 @@ repos:
pass_filenames: true
exclude: ^airflow/_vendor/
additional_dependencies: ['rich>=12.4.4', 'inputimeout']
- id: lint-javascript
name: ESLint against airflow/ui
language: python
'types_or': [javascript, tsx, ts]
files: ^airflow/ui/
entry: ./scripts/ci/pre_commit/pre_commit_ui_lint.py
pass_filenames: false
additional_dependencies: ['rich>=12.4.4', 'inputimeout']
- id: lint-javascript
name: ESLint against current UI JavaScript files
language: python
'types_or': [javascript]
files: ^airflow/www/static/js/
entry: ./scripts/ci/pre_commit/pre_commit_www_lint.py
pass_filenames: false
additional_dependencies: ['rich>=12.4.4', 'inputimeout']
- id: update-migration-references
name: Update migration ref doc
language: python
Expand Down
36 changes: 22 additions & 14 deletions BREEZE.rst
Original file line number Diff line number Diff line change
Expand Up @@ -438,13 +438,30 @@ regenerate all those images (which might be needed in case new version of rich i
:alt: Breeze regenerate-command-images


Compiling www assets
====================

Airflow webserver needs to prepare www assets - compiled with node and yarn. The ``compile-www-assets``
command takes care about it. This is needed when you want to run webserver inside of the breeze.

.. image:: ./images/breeze/output-compile-www-assets.svg
:width: 100%
:alt: Breeze compile-www-assets

Starting complete Airflow installation
======================================

For testing Airflow oyou often want to start multiple components (in multiple terminals). Breeze has
built-in ``start-airflow`` command that start breeze container, launches multiple terminals using tmux
and launches all Airflow necessary components in those terminals.

When you are starting airflow from local sources, www asset compilation is automatically executed before.

.. code-block:: bash
breeze --python 3.7 --backend mysql start-airflow
You can also use it to start any released version of Airflow from ``PyPI`` with the
``--use-airflow-version`` flag.

Expand Down Expand Up @@ -496,6 +513,7 @@ Those are commands mostly used by contributors:
* Execute arbitrary command in the test environment with ``breeze shell`` command
* Enter interactive shell in CI container when ``shell`` (or no command) is specified
* Start containerised, development-friendly airflow installation with ``breeze start-airflow`` command
* Compile www assets for webserver ``breeze compile-www-assets`` command
* Build documentation with ``breeze build-docs`` command
* Initialize local virtualenv with ``./scripts/tools/initialize_virtualenv.py`` command
* Run static checks with autocomplete support ``breeze static-checks`` command
Expand Down Expand Up @@ -1575,24 +1593,14 @@ If you set these variables, next time when you enter the environment the new por
Managing Dependencies
---------------------

If you need to change apt dependencies in the ``Dockerfile.ci``, add Python packages in ``setup.py`` or
add JavaScript dependencies in ``package.json``, you can either add dependencies temporarily for a single
Breeze session or permanently in ``setup.py``, ``Dockerfile.ci``, or ``package.json`` files.

Installing Dependencies for a Single Breeze Session
...................................................

You can install dependencies inside the container using ``sudo apt install``, ``pip install`` or
``yarn install`` (in ``airflow/www`` folder) respectively. This is useful if you want to test something
quickly while you are in the container. However, these changes are not retained: they disappear once you
exit the container (except for the node.js dependencies if your sources are mounted to the container).
Therefore, if you want to retain a new dependency, follow the second option described below.
If you need to change apt dependencies in the ``Dockerfile.ci``, add Python packages in ``setup.py``
for airflow and in provider.yaml for packages. If you add any "node" dependencies in ``airflow/www``
or ``airflow/ui``, you need to compile them in the host with ``breeze compile-www-assets`` command.

Adding Dependencies Permanently
...............................

You can add dependencies to the ``Dockerfile.ci``, ``setup.py`` or ``package.json`` and rebuild the image.
This should happen automatically if you modify any of these files.
You can add dependencies to the ``Dockerfile.ci``, ``setup.py``.
After you exit the container and re-run ``breeze``, Breeze detects changes in dependencies,
asks you to confirm rebuilding the image and proceeds with rebuilding if you confirm (or skip it
if you do not confirm). After rebuilding is done, Breeze drops you to shell. You may also use the
Expand Down
32 changes: 6 additions & 26 deletions CONTRIBUTING.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1162,12 +1162,12 @@ itself comes bundled with jQuery and bootstrap. While they may be phased out
over time, these packages are currently not managed with yarn.

Make sure you are using recent versions of node and yarn. No problems have been
found with node\>=8.11.3 and yarn\>=1.19.1.
found with node\>=8.11.3 and yarn\>=1.19.1. The pre-commit framework of ours install
node and yarn automatically when installed - if you use ``breeze`` you do not need to install
neither node nor yarn.

Installing yarn and its packages
--------------------------------

Make sure yarn is available in your environment.
Installing yarn and its packages manually
-----------------------------------------

To install yarn on macOS:

Expand All @@ -1191,27 +1191,6 @@ To install yarn on macOS:
export PATH="$HOME/.yarn/bin:$PATH"
4. Install third-party libraries defined in ``package.json`` by running the
following commands within the ``airflow/www/`` directory:


.. code-block:: bash
# from the root of the repository, move to where our JS package.json lives
cd airflow/www/
# run yarn install to fetch all the dependencies
yarn install
These commands install the libraries in a new ``node_modules/`` folder within
``www/``.

Should you add or upgrade a node package, run
``yarn add --dev <package>`` for packages needed in development or
``yarn add <package>`` for packages used by the code.
Then push the newly generated ``package.json`` and ``yarn.lock`` file so that we
could get a reproducible build. See the `Yarn docs
<https://yarnpkg.com/en/docs/cli/add#adding-dependencies->`_ for more details.


Generate Bundled Files with yarn
--------------------------------
Expand All @@ -1225,6 +1204,7 @@ commands:
yarn run prod
# Starts a web server that manages and updates your assets as you modify them
# You'll need to run the webserver in debug mode too: ``airflow webserver -d``
yarn run dev
Expand Down
Loading

0 comments on commit 60bc1ce

Please sign in to comment.