-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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 dev version of asset compilation #25272
Conversation
0ee2cf7
to
443e14a
Compare
253a104
to
07f82bf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 👍
07f82bf
to
2431a1e
Compare
BTW. @bbovenzi @pierrejeambrun - one comment, the nice thing about using pre-commit is that you do not have to care about maintaining your 'node + yarn' in the right version, so it will be always "working the same" for everyone using But the problem with pre-commit is that it captures the outpt and only shows it (even in verbose mode) after it completes, but in this case - it will never complete :) - so you do not see the output of the compiled assets as they change (which I find is a good "sanity check" to see that things are atually being refreshed. You will see it in |
ceb3654
to
18a5381
Compare
Yeah, why not. |
When we moved www asset compilation from container to the host in apache#25169, we removed yarn from the image and the old instructions are no longer valid. Instead of in-container yarn dev we expect now to have yarn dev run in the host. It can be done either with pre-commit (if you have no node/yarn installed in your host, pre-commit will automatically install both node and yarn in the right versions, or if you have yarn installed you can run it manually. The 'start-airflow' instructions now remove the in-container instructions and explain those two options you have.
18a5381
to
3e374dc
Compare
How about this 😛 @bbovenzi ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
When we moved www asset compilation from container to the host in #25169, we removed yarn from the image and the old instructions are no longer valid. Instead of in-container yarn dev we expect now to have yarn dev run in the host. It can be done either with pre-commit (if you have no node/yarn installed in your host, pre-commit will automatically install both node and yarn in the right versions, or if you have yarn installed you can run it manually. The 'start-airflow' instructions now remove the in-container instructions and explain those two options you have. (cherry picked from commit 1544868)
When we moved www asset compilation from container to the host in #25169,
we removed yarn from the image and the old instructions are no longer
valid. Instead of in-container yarn dev we expect now
to have yarn dev run in the host. It can be done either with
pre-commit (if you have no node/yarn installed in your host, pre-commit
will automatically install both node and yarn in the right versions,
or if you have yarn installed you can run it manually.
The 'start-airflow' instructions now remove the in-container
instructions and explain those two options you have.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
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.