Skip to content

Add utility function for generic filter counting #54817

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add utility function for generic filter counting #54817

wants to merge 2 commits into from
+135 −101

Conversation

guan404ming
Copy link
Contributor

Related

#54671 (comment)

Why

  • There are several filter count logic could be reused.
  • Lots of filters will be added in 3.1.0 and most of them would also need the function to count filter

How

  • centralize filter counting logic
  • dynamically count filters and preserve type safe by TypeScript generic
  • add unit test for the new func

^ 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 airflow-core/newsfragments.

@boring-cyborg boring-cyborg bot added the area:UI Related to UI/UX. For Frontend Developers. label Aug 22, 2025
@guan404ming guan404ming mentioned this pull request Aug 22, 2025
Merged
pierrejeambrun
pierrejeambrun approved these changes Aug 22, 2025
View reviewed changes
Copy link
Member

@pierrejeambrun pierrejeambrun left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice, I don't think we need the excluded fields for now, I would just remove this to not over complicate it.

We can always add that back later on if needed. This will also make the function easier to type I believe.

@guan404ming guan404ming force-pushed the refactor-filter-utils branch from a585841 to c14a4af Compare August 22, 2025 14:23
@guan404ming
Copy link
Contributor Author

guan404ming commented Aug 22, 2025 •
edited
Loading

Nice, I don't think we need the excluded fields for now, I would just remove this to not over complicate it. We can always add that back later on if needed. This will also make the function easier to type I believe.

Sure, it would be much easier to type. Current removed and I could help update it if needed in the future!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pierrejeambrun pierrejeambrun pierrejeambrun approved these changes

@bbovenzi bbovenzi Awaiting requested review from bbovenzi bbovenzi is a code owner

@ryanahamilton ryanahamilton Awaiting requested review from ryanahamilton ryanahamilton is a code owner

@jscheffl jscheffl Awaiting requested review from jscheffl jscheffl is a code owner

@shubhamraj-git shubhamraj-git Awaiting requested review from shubhamraj-git shubhamraj-git is a code owner

Assignees
No one assigned
Labels
area:UI Related to UI/UX. For Frontend Developers.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@guan404ming @pierrejeambrun