You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "bbovenzi (via GitHub)" <gi...@apache.org> on 2024/08/28 18:39:29 UTC
[PR] Initialize new airflow/ui directory [airflow]
bbovenzi opened a new pull request, #41846:
URL: https://github.com/apache/airflow/pull/41846
For Airflow 3, we want a fully React UI. The first step is to create a new UI project at `airflow/ui`.
New tech:
- vite
Framework to build the react app
- pnpm
CLI command to install packages, handles dependencies better than yarn and npm
Banner telling users to try the new UI:
<img width="522" alt="Screenshot 2024-08-27 at 2 56 59 PM" src="https://github.com/user-attachments/assets/7bf7f947-9aba-4e69-b449-d9b1fd38975b">
New UI with the ability to go back to the old UI:
<img width="1084" alt="Screenshot 2024-08-27 at 2 56 52 PM" src="https://github.com/user-attachments/assets/1fb7c846-c413-4534-96f5-c21759862965">
Hot reload works when running `breeze start-airflow --dev-mode` or manipulating the `dev_mode` env var and running the vite server manually.
We could spend a lot more time improving precommit and breeze config. But I'd like to not get too bogged in that and get this project started.
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.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
Re: [PR] Initialize new airflow/ui directory [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #41846:
URL: https://github.com/apache/airflow/pull/41846#issuecomment-2317927455
> Interesting thanks Jarek for the explanation, I think we can completely remove it in that PR, as there will be no python tests in `/ui` but I will definitely take the comment for the `/api_ui` folder. (That I am working on atm)
In this case likely `API test type should just be extended in the MAP to include that folder then - then the tests will be executed as part of the `API` test type.
```
"API": ["tests/api", "tests/api_connexion", "tests/api_internal"],
```
--
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
Re: [PR] Initialize new airflow/ui directory [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #41846:
URL: https://github.com/apache/airflow/pull/41846#discussion_r1736192777
##########
.pre-commit-config.yaml:
##########
@@ -19,7 +19,7 @@ default_stages: [commit, push]
default_language_version:
python: python3
node: 22.2.0
-minimum_pre_commit_version: '3.2.0'
Review Comment:
Is it possible to revert those quote changes ? I guess it's vscode applying it automatically ?
--
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
Re: [PR] Initialize new airflow/ui directory [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #41846:
URL: https://github.com/apache/airflow/pull/41846#issuecomment-2316880603
Nice, looking good overall.
I'll let others take a look before approving, especially for non ui related part such as the CI integration, selectives checks and Breeze. (Which look good but could use a second pair of eyes)
--
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
Re: [PR] Initialize new airflow/ui directory [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on PR #41846:
URL: https://github.com/apache/airflow/pull/41846#issuecomment-2319945429
Should we add something in `contributing-docs` on how to run the tests too? It can be very short for now, just so people diving into the code base know the minium what to do.
--
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
Re: [PR] Initialize new airflow/ui directory [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #41846:
URL: https://github.com/apache/airflow/pull/41846#issuecomment-2317731029
Looks good @bbovenzi - I saw you changed www compilation to happen in foreground to fix running two background pre-commits at the same time - that should do at the expense of slower startup time when www change but that should be rare if at all now).
Few coments on failing tests:
* All failing selective check tests - there is no need to add UI to SelectiveUnitTtestType at all - there are no "python" tests in "tests/ui" - those unit tests are python -only - WWW "javascript" tests are run separately (React WWW). Those "WWW" tests are python tests for "airflow/www/*.py` files. This is the reason why the UI tests are "hanging"
As I understand Python code is still going to be in "airflow/www" ? If yes - just re
If want to have some python tests in "tests/ui" - the `"UI": ["tests/ui"]` entry should be added to this map (in `airflow_breeze/utils/run_tests.py` - where type of test is mapped to the pytest command executed (and some "empty" test should be added in "tests/ui"):
```python
TEST_TYPE_MAP_TO_PYTEST_ARGS: dict[str, list[str]] = {
...
"WWW": [
"tests/www",
],
}
```
* React UI tests failing - `pnpm` needs to be installed after "setup-node" step - as we are running the tests without Breeze CI image
* Test Open API Client - should be already fixed in main by #41858
* Static checks - just re-basing + re-running pre-commit should fix them
--
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
Re: [PR] Initialize new airflow/ui directory [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #41846:
URL: https://github.com/apache/airflow/pull/41846#issuecomment-2320691741
Just added a small commit to try to fix the static checks, lets see if the CI is happy 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
Re: [PR] Initialize new airflow/ui directory [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi merged PR #41846:
URL: https://github.com/apache/airflow/pull/41846
--
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
Re: [PR] Initialize new airflow/ui directory [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #41846:
URL: https://github.com/apache/airflow/pull/41846#discussion_r1736192777
##########
.pre-commit-config.yaml:
##########
@@ -19,7 +19,7 @@ default_stages: [commit, push]
default_language_version:
python: python3
node: 22.2.0
-minimum_pre_commit_version: '3.2.0'
Review Comment:
Is it possible to revert that quote change ? I guess it's vscode applying it automatically ?
--
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
Re: [PR] Initialize new airflow/ui directory [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #41846:
URL: https://github.com/apache/airflow/pull/41846#issuecomment-2317916557
Interesting thanks Jarek for the explanation, I think we can completely remove it in that PR, as there will be no python tests in `/ui` but I will definitely take the comment for the `/api_ui` folder. (That I am working on atm)
--
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
Re: [PR] Initialize new airflow/ui directory [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #41846:
URL: https://github.com/apache/airflow/pull/41846#discussion_r1736264646
##########
.pre-commit-config.yaml:
##########
@@ -19,7 +19,7 @@ default_stages: [commit, push]
default_language_version:
python: python3
node: 22.2.0
-minimum_pre_commit_version: '3.2.0'
Review Comment:
BTW. Likely we should apply some lint rule here by pre-commit - not sure if yamllint can be configured to expect `"`
--
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
Re: [PR] Initialize new airflow/ui directory [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #41846:
URL: https://github.com/apache/airflow/pull/41846#issuecomment-2321078279
Looks good
--
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
Re: [PR] Initialize new airflow/ui directory [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #41846:
URL: https://github.com/apache/airflow/pull/41846#issuecomment-2321164255
Yep. Looks like ready to merge.
--
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