jscheffl commented on code in PR #45060:
URL: https://github.com/apache/airflow/pull/45060#discussion_r1890912675
##########
scripts/ci/pre_commit/compile_www_assets.py:
##########
@@ -71,4 +72,23 @@ def get_directory_hash(directory: Path, skip_path_regexp:
str | None = None) ->
subprocess.check_call(["yarn", "install", "--frozen-lockfile"],
cwd=os.fspath(www_directory))
subprocess.check_call(["yarn", "run", "build"],
cwd=os.fspath(www_directory), env=env)
new_hash = get_directory_hash(www_directory,
skip_path_regexp=r".*node_modules.*")
- WWW_HASH_FILE.write_text(new_hash)
+ www_hash_file.write_text(new_hash)
+
+
+def is_fab_provider_installed() -> bool:
+ return (
+ importlib.util.find_spec("airflow") is not None
Review Comment:
I thought from the point of lifecycle. The pre-commit hook will be called in
CI and locally... at build time. Source tree will be there. And it _needs_ to
be called at build-time to create the static JS content as dist folder to be
packaged into the wheel.
The check you are referring to is at _runtime_ At `pip install
apache-airflow-providers-fab` time the wheel is only un-packed to
site-packages. There will no pre-commit be executed as well as you need to
assume that at point of install no yarn will be available (might be the user
installing is even offline w/o internet).
Otherwise - not being an expert - if the wheel install need to
compile/bind/do something at point of install you need to define this in
package meta data - not in a pre-commit script that is not available at install
time.
You can see this also for airflow/www - the pre-commit is making the package
_before_ the sdist/wheel is built and then the dist is packed into the wheel.
So if the check now makes it more safe... I assume the dist will just not be
created during build then and only the TS/TSX source tree will be packaged into
the wheel.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
