-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Add --mount-ui-dist flag for Breeze start-airflow command #59778
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
Conversation
amoghrajesh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
|
Very cool idea! But it can be done way simpler I think. You can likely directly mount the |
gopidesupavan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice addition :)
jason810496
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. I thinl rather than doing this copy, we could simply mount the folder directly where it is expected to be? I don't think we need to use the temporary folder to copy it from there inside?
Yes, very agree with that! It's way more better to mount it directly to library path.
Co-authored-by: Amogh Desai <[email protected]>
breeze setup regenerate-command-images --force
e296418 to
814b3bf
Compare
jason810496
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks all for the review! I just refactored as directly mount to correct path instead of copying from tmp path.
shahar1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a great feature to have, one comment though :)
Backport failed to create: v3-1-test. View the failure log Run details
You can attempt to backport this manually by running: cherry_picker 01fe16d v3-1-testThis should apply the commit to the v3-1-test branch and leave the commit in conflict state marking After you have resolved the conflicts, you can continue the backport process by running: cherry_picker --continue |
* Add --mount-ui-dist flag for Breeze start-airflow * Update dev/breeze/src/airflow_breeze/commands/common_options.py Co-authored-by: Amogh Desai <[email protected]> * Directly mount UI dist to library path breeze setup regenerate-command-images --force * breeze setup regenerate-command-images --force * Use PYTHON_MAJOR_MINOR_VERSION env instead of hardcoded 3.10 --------- Co-authored-by: Amogh Desai <[email protected]>
* Add --mount-ui-dist flag for Breeze start-airflow * Update dev/breeze/src/airflow_breeze/commands/common_options.py Co-authored-by: Amogh Desai <[email protected]> * Directly mount UI dist to library path breeze setup regenerate-command-images --force * breeze setup regenerate-command-images --force * Use PYTHON_MAJOR_MINOR_VERSION env instead of hardcoded 3.10 --------- Co-authored-by: Amogh Desai <[email protected]>
* Add --mount-ui-dist flag for Breeze start-airflow * Update dev/breeze/src/airflow_breeze/commands/common_options.py Co-authored-by: Amogh Desai <[email protected]> * Directly mount UI dist to library path breeze setup regenerate-command-images --force * breeze setup regenerate-command-images --force * Use PYTHON_MAJOR_MINOR_VERSION env instead of hardcoded 3.10 --------- Co-authored-by: Amogh Desai <[email protected]>
related: #57219
Why
After Auto-compile UI assets on Breeze start-airflow command #57219, we could
breeze start-airflow --use-airflow version <pr-number> / <owner/repo:branch>and access the Frontend successfully. We make it happen by:node.jsnpmpnpmpnpm build/usr/python/lib/python3.10/site-packages/airflow/ui/distHowever, if we just want to test the Airflow Core change from someone's PR (e.g. validate API Server TaskInstance Log reading path, check Scheduler behavior but still want to access Frontend for observability, etc), we still need to do the all the steps above just to access Frontend, which is waste of time for test iteration.
Instead, we could just mount the pre-built UI dist from host to Breeze container to save the time, in most of the time, the core change will not impact API Server <-> Frontend behavior. (e.g. When I testing Fix rendering of template fields with start from trigger#55068 recently, there isn't anything related with Frontend, but I still need to access it to check the TaskInstance status. )
What
Add
--mount-ui-distflag forstart-airflowcommand to mount UI dist directly from host to Breeze container without building the whole frontend again to save time.Verfication
--mount-ui-dist, the middle of it should be navy blue color instead of pure black if we are using3.1.5--mount-ui-distOutput of
breeze start-airflow --backend postgres --mount-sources providers-and-tests --use-airflow-version apache/airflow:main --mount-ui-dist