Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Jun 14, 2025

Running pre-commits for typescript compilation, linting formatting is currently very slow. The pre-commit that is doing it took more than 2.5 minutes - always, independently which files changed. Also, no matter whether UI or simple auth manager UI changed, both were compiled.

This is the first iteration of speeding up the pre-commit especially for incremental changes.

The following changes were implemented:

  • the scripts were split into ui and simple-auth-manager-ui
  • timing information and verbose commands displayed so that we can measure which commands take how much time
  • the pre-commmit now passes all the files as input (and pre-commit is serialized) - which means that some commands can only be run on the subset of files that changed
  • the steps in pre-commit are not executed if relevant files have not changed
  • tsc step is modified to only run on a subset of .ts and .tsx files that were changed (via dynamic extension of the project)

Overall when just few files change, the pre-commit should now complete in less than 10-20 seconds

Still some optimisations are possible in the future.


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

Running pre-commits for typescript compilation, linting formatting is
currently very slow. The pre-commit that is doing it took more than 2.5
minutes - always, independently which files changed. Also, no matter
whether UI or simple auth manager UI changed, both were compiled.

This is the first iteration of speeding up the pre-commit especially
for incremental changes.

The following changes were implemented:

* the scripts were split into ui and simple-auth-manager-ui
* timing information and verbose commands displayed so that
  we can measure which commands take how much time
* the pre-commmit now passes all the files as input (and pre-commit
  is serialized) - which means that some commands can only be run
  on the subset of files that changed
* tsc step is modified to only run on a subset of .ts and .tsx files
  that were changed (via dynamic extension of the project)

Overall when just few files change, the pre-commit should now complete
in less than 10-20 seconds

Still some optimisations are possible in the future.
@potiuk
Copy link
Member Author

potiuk commented Jun 14, 2025

We could - potentially already refactor the code in both pre-commits - because they are essentially the same (minus directory) - but I think maybe coupling those two might not make sense eventually - we might want to have different steps in both (but happy to hear your opinion).

@potiuk potiuk added this to the Airflow 3.0.3 milestone Jun 14, 2025
@potiuk potiuk added the backport-to-v3-1-test Mark PR with this label to backport to v3-1-test branch label Jun 14, 2025
Copy link
Contributor

@amoghrajesh amoghrajesh left a comment

Choose a reason for hiding this comment

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

I like it!

@potiuk potiuk merged commit 52be0dd into apache:main Jun 14, 2025
99 checks passed
@potiuk potiuk deleted the optimize-ui-lint-pre-commits branch June 14, 2025 13:25
@github-actions
Copy link

Backport failed to create: v3-0-test. View the failure log Run details

Status Branch Result
v3-0-test Commit Link

You can attempt to backport this manually by running:

cherry_picker 52be0dd v3-0-test

This should apply the commit to the v3-0-test branch and leave the commit in conflict state marking
the files that need manual conflict resolution.

After you have resolved the conflicts, you can continue the backport process by running:

cherry_picker --continue

potiuk added a commit to potiuk/airflow that referenced this pull request Jun 14, 2025
Running pre-commits for typescript compilation, linting formatting is
currently very slow. The pre-commit that is doing it took more than 2.5
minutes - always, independently which files changed. Also, no matter
whether UI or simple auth manager UI changed, both were compiled.

This is the first iteration of speeding up the pre-commit especially
for incremental changes.

The following changes were implemented:

* the scripts were split into ui and simple-auth-manager-ui
* timing information and verbose commands displayed so that
  we can measure which commands take how much time
* the pre-commmit now passes all the files as input (and pre-commit
  is serialized) - which means that some commands can only be run
  on the subset of files that changed
* tsc step is modified to only run on a subset of .ts and .tsx files
  that were changed (via dynamic extension of the project)

Overall when just few files change, the pre-commit should now complete
in less than 10-20 seconds

Still some optimisations are possible in the future.
(cherry picked from commit 52be0dd)

Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
potiuk added a commit that referenced this pull request Jun 14, 2025
Running pre-commits for typescript compilation, linting formatting is
currently very slow. The pre-commit that is doing it took more than 2.5
minutes - always, independently which files changed. Also, no matter
whether UI or simple auth manager UI changed, both were compiled.

This is the first iteration of speeding up the pre-commit especially
for incremental changes.

The following changes were implemented:

* the scripts were split into ui and simple-auth-manager-ui
* timing information and verbose commands displayed so that
  we can measure which commands take how much time
* the pre-commmit now passes all the files as input (and pre-commit
  is serialized) - which means that some commands can only be run
  on the subset of files that changed
* tsc step is modified to only run on a subset of .ts and .tsx files
  that were changed (via dynamic extension of the project)

Overall when just few files change, the pre-commit should now complete
in less than 10-20 seconds

Still some optimisations are possible in the future.
(cherry picked from commit 52be0dd)
Copy link
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

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

Cool!

@potiuk
Copy link
Member Author

potiuk commented Jun 14, 2025

Yeah - it should take seconds now for translators :)

@bbovenzi
Copy link
Contributor

Thank you! I've been meaning to look into why it was so slow.

@potiuk
Copy link
Member Author

potiuk commented Jun 14, 2025

Thank you! I've been meaning to look into why it was so slow.

You are most welcome .... Also It would be good to see if it works "perfectly" - I think I got all the types of files in and trigger the right things when the "source" ones change - but I might have missed "some" edge cases file types.

But it should be easy to update the pre-commits to fix any changes so feel free in case you see anythign missed.

We could also potentially add a manual pre-commit similar to what was before to retrigger it "for sure" and without passing the files to pre-commit in case we want to be absolutely sure. We had similar setup for mypy in the past.

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

Labels

area:dev-tools backport-to-v3-1-test Mark PR with this label to backport to v3-1-test branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants