Re: [PR] AIP-79 Generate assets for Flask application in FAB provider [airflow]

2024-12-18 Thread via GitHub


jscheffl commented on code in PR #44744:
URL: https://github.com/apache/airflow/pull/44744#discussion_r1890818365


##
scripts/ci/pre_commit/compile_www_assets.py:
##
@@ -71,4 +72,20 @@ 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.providers.fab") is not None

Review Comment:
   Propose to fix via #45058



-- 
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-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] AIP-79 Generate assets for Flask application in FAB provider [airflow]

2024-12-18 Thread via GitHub


jscheffl commented on code in PR #44744:
URL: https://github.com/apache/airflow/pull/44744#discussion_r1890792030


##
scripts/ci/pre_commit/compile_www_assets.py:
##
@@ -71,4 +72,20 @@ 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.providers.fab") is not None

Review Comment:
   Ups, this change in this function broke main :-( 
   https://github.com/apache/airflow/actions/runs/12399970802/job/34616478712
   
   I assume we need to find another method to check if FAB is installed.



-- 
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-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] AIP-79 Generate assets for Flask application in FAB provider [airflow]

2024-12-18 Thread via GitHub


vincbeck merged PR #44744:
URL: https://github.com/apache/airflow/pull/44744


-- 
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-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] AIP-79 Generate assets for Flask application in FAB provider [airflow]

2024-12-11 Thread via GitHub


vincbeck commented on PR #44744:
URL: https://github.com/apache/airflow/pull/44744#issuecomment-2536425607

   > Also it seems there is already some plumbing and automations set up to 
generate the assets automatically when creating a virtual env with hatch so I 
think I'll keep everything in that file. That will simplify things. I added a 
test in the script to do it only if the fab provider package is installed
   
   @jedcunningham WDYT?


-- 
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-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] AIP-79 Generate assets for Flask application in FAB provider [airflow]

2024-12-09 Thread via GitHub


vincbeck commented on PR #44744:
URL: https://github.com/apache/airflow/pull/44744#issuecomment-2528560275

   Also it seems there is already some plumbing and automations set up to 
generate the assets automatically when creating a virtual env with hatch so I 
think I'll keep everything in that file. That will simplify things. I added a 
test in the script to do it only if the provider package is installed


-- 
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-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] AIP-79 Generate assets for Flask application in FAB provider [airflow]

2024-12-09 Thread via GitHub


vincbeck commented on PR #44744:
URL: https://github.com/apache/airflow/pull/44744#issuecomment-2528543670

   > > Oh, that probably is a better idea.
   > > These are the docs for the release process: 
https://github.com/apache/airflow/blob/main/dev/README_RELEASE_PROVIDER_PACKAGES.md
   > 
   > Cool :) I'll update the PR
   
   I just realized, the release should not be updated because the assets are 
not released. It should be in the install process


-- 
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-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] AIP-79 Generate assets for Flask application in FAB provider [airflow]

2024-12-09 Thread via GitHub


vincbeck commented on PR #44744:
URL: https://github.com/apache/airflow/pull/44744#issuecomment-2528345118

   > Oh, that probably is a better idea.
   > 
   > These are the docs for the release process: 
https://github.com/apache/airflow/blob/main/dev/README_RELEASE_PROVIDER_PACKAGES.md
   
   Cool :) I'll update the PR


-- 
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-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] AIP-79 Generate assets for Flask application in FAB provider [airflow]

2024-12-06 Thread via GitHub


jedcunningham commented on PR #44744:
URL: https://github.com/apache/airflow/pull/44744#issuecomment-2524940846

   Oh, that probably is a better idea.
   
   These are the docs for the release process: 
https://github.com/apache/airflow/blob/main/dev/README_RELEASE_PROVIDER_PACKAGES.md


-- 
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-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] AIP-79 Generate assets for Flask application in FAB provider [airflow]

2024-12-06 Thread via GitHub


vincbeck commented on PR #44744:
URL: https://github.com/apache/airflow/pull/44744#issuecomment-2524060936

   I just had a double thought. Would not it be better to copy paste this 
script in FAB provider and generate only the assets for FAB provider in this 
script, update the release process of FAB provider to run this script? That 
way, when we remove the entire Flash application from core, we can safely 
remove the script from core?


-- 
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-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] AIP-79 Generate assets for Flask application in FAB provider [airflow]

2024-12-06 Thread via GitHub


vincbeck commented on PR #44744:
URL: https://github.com/apache/airflow/pull/44744#issuecomment-2524059146

   > Overall, lgtm. Do we need to add a step to run this into the provider 
release process for FAB?
   
   I think so. Do you know where is this provider release process?


-- 
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-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org