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/07/25 07:47:37 UTC
[GitHub] [airflow] potiuk opened a new pull request, #25272: Add dev version of asset compilation
potiuk opened a new pull request, #25272:
URL: https://github.com/apache/airflow/pull/25272
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.
<!--
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+Improvements+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 pull request #25272: Add dev version of asset compilation
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25272:
URL: https://github.com/apache/airflow/pull/25272#issuecomment-1194503878
How about this :stuck_out_tongue: @bbovenzi ?
--
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] bbovenzi commented on pull request #25272: Add dev version of asset compilation
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on PR #25272:
URL: https://github.com/apache/airflow/pull/25272#issuecomment-1194487065
<img width="517" alt="Screen Shot 2022-07-25 at 7 58 12 PM" src="https://user-images.githubusercontent.com/4600967/180853483-fbbc9d9e-c3aa-43cb-bc01-1905d1636daa.png">
I wonder if we should add something to the message here so that developers know that its actively running and not stuck?
--
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 #25272: Add dev version of asset compilation
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25272:
URL: https://github.com/apache/airflow/pull/25272#issuecomment-1194490702
Yeah, why not.
--
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] bbovenzi commented on a diff in pull request #25272: Add dev version of asset compilation
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on code in PR #25272:
URL: https://github.com/apache/airflow/pull/25272#discussion_r928645232
##########
dev/breeze/src/airflow_breeze/commands/developer_commands.py:
##########
@@ -558,13 +566,20 @@ def static_checks(
allow_extra_args=True,
),
)
+@click.option(
+ "--dev",
+ help="Run development version of assets compilation - it will not quit and automatically "
+ "recompile assets on-the-flight when they are changed.",
Review Comment:
```suggestion
"recompile assets on-the-fly when they are changed.",
```
--
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] pierrejeambrun commented on pull request #25272: Add dev version of asset compilation
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on PR #25272:
URL: https://github.com/apache/airflow/pull/25272#issuecomment-1193957243
Great
--
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 #25272: Add dev version of asset compilation
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25272:
URL: https://github.com/apache/airflow/pull/25272#issuecomment-1193980561
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 `breeze` and `pre-commit` under the hood.
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 "webserver -d` output, so it's not a big problem, and you can likely live with it, but I decided to mention also the option of runing `yarn dev` manually as it gives instant feedback about stuff being recompiled.
--
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 #25272: Add dev version of asset compilation
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #25272:
URL: https://github.com/apache/airflow/pull/25272
--
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