Skip to content
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

AIP-38 | List Import Errors #44021

Merged
merged 16 commits into from
Nov 18, 2024
Merged

AIP-38 | List Import Errors #44021

merged 16 commits into from
Nov 18, 2024

Conversation

shubhamraj-git
Copy link
Contributor

@shubhamraj-git shubhamraj-git commented Nov 14, 2024

closes: #43711

Screenshot 2024-11-16 at 2 13 09 AM Screenshot 2024-11-16 at 2 13 18 AM Screenshot 2024-11-16 at 2 13 57 AM Screenshot 2024-11-16 at 2 14 24 AM

^ 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.

@boring-cyborg boring-cyborg bot added the area:UI Related to UI/UX. For Frontend Developers. label Nov 14, 2024
@tirkarthi
Copy link
Contributor

One point to add is that current import errors are shown all at once in the legacy home page which makes using Ctrl+F easier to search especially on large Airflow instances with lot of users and hundreds of import errors which might not be possible anymore with pagination.

Copy link
Contributor

@bbovenzi bbovenzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great start! I left a few comments that should make this PR simpler.

airflow/ui/src/pages/Dashboard/Dashboard.tsx Outdated Show resolved Hide resolved
airflow/ui/src/queries/useDagsImportErrors.tsx Outdated Show resolved Hide resolved
airflow/ui/src/pages/Dashboard/Stats/DAGImportErrors.tsx Outdated Show resolved Hide resolved
airflow/ui/src/pages/Dashboard/Stats/Stats.tsx Outdated Show resolved Hide resolved
@shubhamraj-git
Copy link
Contributor Author

One point to add is that current import errors are shown all at once in the legacy home page which makes using Ctrl+F easier to search especially on large Airflow instances with lot of users and hundreds of import errors which might not be possible anymore with pagination.

@tirkarthi I have added a search option to do that.

@shubhamraj-git
Copy link
Contributor Author

@bbovenzi
Also, since this is an error showing page, I saw the old UI have names too in red, So what do you think, which of the two options look good? Red or normal black?
image

Copy link
Contributor

@bbovenzi bbovenzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can keep it black, you already know its an error.

Great work!

@bbovenzi bbovenzi merged commit 8027055 into apache:main Nov 18, 2024
37 checks passed
amoghrajesh pushed a commit to astronomer/airflow that referenced this pull request Nov 18, 2024
* create skeleton

* added modal

* add data in modal

* refactor

* time stamp

* handling reviews

* colour fix and search

* add skeleton and colour fix

* add error

* handle reviews
kandharvishnu pushed a commit to kandharvishnu/airflow that referenced this pull request Nov 19, 2024
* create skeleton

* added modal

* add data in modal

* refactor

* time stamp

* handling reviews

* colour fix and search

* add skeleton and colour fix

* add error

* handle reviews
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:UI Related to UI/UX. For Frontend Developers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AIP-38 | List Import Errors
3 participants