-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
Make graphviz
dependency optional
#36647
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
boring-cyborg
bot
added
area:dev-tools
area:production-image
Production image improvements and fixes
kind:documentation
labels
Jan 7, 2024
potiuk
requested review from
pankajkoti,
jscheffl,
ferruzzi,
Taragolis and
hussein-awala
January 7, 2024 13:20
potiuk
force-pushed
the
make-graphviz-dependency-optional
branch
from
January 7, 2024 13:26
e700c8c
to
643e98a
Compare
jscheffl
approved these changes
Jan 7, 2024
The `graphviz` dependency has been problematic as Airflow required dependency - especially for ARM-based installations. Graphviz packages require binary graphviz libraries - which is already a limitation, but they also require to install graphviz Python bindings to be build and installed. This does not work for older Linux installation but - more importantly - when you try to install Graphviz libraries for Python 3.8, 3.9 for ARM M1 MacBooks, the packages fail to install because Python bindings compilation for M1 can only work for Python 3.10+. There is not an easy solution for that except commenting out graphviz dependency from setup.py, when you want to install Airflow for Python 3.8, 3.9 for MacBook M1. However Graphviz is really used in two places: * when you want to render DAGs wia airflow CLI - either to an image or directly to terminal (for terminals/systems supporting imgcat) * when you want to render ER diagram after you modified Airflow models The latter is a development-only feature, the former is production feature, however it is a very niche one. This PR turns rendering of the images in Airflow in optional feature (only working when graphviz python bindings are installed) and effectively turns graphviz into an optional extra (and removes it from requirements). This is not a breaking change technically - the CLIs to render the DAGs is still there and IF you already have graphviz installed, it will continue working as it did before. The only problem when it does not work is where you do not have graphviz installed for fresh installation and it will raise an error and inform that you need it. Graphviz will remain to be installed for most users: * the Airflow Image will still contain graphviz library, because it is added there as extra * when previous version of Airflow has been installed already, then graphviz library is already installed there and Airflow will continue working as it did The only change will be a new installation of new version of Airflow from the scratch, where graphviz will need to be specified as extra or installed separately in order to enable DAG rendering option. Taking into account this behaviour (which only requires to install a graphviz package), this should not be considered as a breaking change. Extracted from: apache#36537
potiuk
force-pushed
the
make-graphviz-dependency-optional
branch
from
January 7, 2024 14:59
643e98a
to
5d39632
Compare
ephraimbuddy
added
the
type:misc/internal
Changelog: Misc changes that should appear in change log
label
Jan 10, 2024
potiuk
added
changelog:skip
Changes that should be skipped from the changelog (CI, tests, etc..)
and removed
changelog:skip
Changes that should be skipped from the changelog (CI, tests, etc..)
labels
Jan 13, 2024
potiuk
added a commit
that referenced
this pull request
Jan 13, 2024
The `graphviz` dependency has been problematic as Airflow required dependency - especially for ARM-based installations. Graphviz packages require binary graphviz libraries - which is already a limitation, but they also require to install graphviz Python bindings to be build and installed. This does not work for older Linux installation but - more importantly - when you try to install Graphviz libraries for Python 3.8, 3.9 for ARM M1 MacBooks, the packages fail to install because Python bindings compilation for M1 can only work for Python 3.10+. There is not an easy solution for that except commenting out graphviz dependency from setup.py, when you want to install Airflow for Python 3.8, 3.9 for MacBook M1. However Graphviz is really used in two places: * when you want to render DAGs wia airflow CLI - either to an image or directly to terminal (for terminals/systems supporting imgcat) * when you want to render ER diagram after you modified Airflow models The latter is a development-only feature, the former is production feature, however it is a very niche one. This PR turns rendering of the images in Airflow in optional feature (only working when graphviz python bindings are installed) and effectively turns graphviz into an optional extra (and removes it from requirements). This is not a breaking change technically - the CLIs to render the DAGs is still there and IF you already have graphviz installed, it will continue working as it did before. The only problem when it does not work is where you do not have graphviz installed for fresh installation and it will raise an error and inform that you need it. Graphviz will remain to be installed for most users: * the Airflow Image will still contain graphviz library, because it is added there as extra * when previous version of Airflow has been installed already, then graphviz library is already installed there and Airflow will continue working as it did The only change will be a new installation of new version of Airflow from the scratch, where graphviz will need to be specified as extra or installed separately in order to enable DAG rendering option. Taking into account this behaviour (which only requires to install a graphviz package), this should not be considered as a breaking change. Extracted from: #36537 (cherry picked from commit 89f1737)
ephraimbuddy
pushed a commit
that referenced
this pull request
Jan 15, 2024
The `graphviz` dependency has been problematic as Airflow required dependency - especially for ARM-based installations. Graphviz packages require binary graphviz libraries - which is already a limitation, but they also require to install graphviz Python bindings to be build and installed. This does not work for older Linux installation but - more importantly - when you try to install Graphviz libraries for Python 3.8, 3.9 for ARM M1 MacBooks, the packages fail to install because Python bindings compilation for M1 can only work for Python 3.10+. There is not an easy solution for that except commenting out graphviz dependency from setup.py, when you want to install Airflow for Python 3.8, 3.9 for MacBook M1. However Graphviz is really used in two places: * when you want to render DAGs wia airflow CLI - either to an image or directly to terminal (for terminals/systems supporting imgcat) * when you want to render ER diagram after you modified Airflow models The latter is a development-only feature, the former is production feature, however it is a very niche one. This PR turns rendering of the images in Airflow in optional feature (only working when graphviz python bindings are installed) and effectively turns graphviz into an optional extra (and removes it from requirements). This is not a breaking change technically - the CLIs to render the DAGs is still there and IF you already have graphviz installed, it will continue working as it did before. The only problem when it does not work is where you do not have graphviz installed for fresh installation and it will raise an error and inform that you need it. Graphviz will remain to be installed for most users: * the Airflow Image will still contain graphviz library, because it is added there as extra * when previous version of Airflow has been installed already, then graphviz library is already installed there and Airflow will continue working as it did The only change will be a new installation of new version of Airflow from the scratch, where graphviz will need to be specified as extra or installed separately in order to enable DAG rendering option. Taking into account this behaviour (which only requires to install a graphviz package), this should not be considered as a breaking change. Extracted from: #36537 (cherry picked from commit 89f1737)
68 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
area:dev-tools
area:production-image
Production image improvements and fixes
kind:documentation
type:misc/internal
Changelog: Misc changes that should appear in change log
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
graphviz
dependency has been problematic as Airflow required dependency - especially for ARM-based installations. Graphviz packages require binary graphviz libraries - which is already a limitation, but they also require to install graphviz Python bindings to be build and installed. This does not work for older Linux installation but - more importantly - when you try to install Graphviz libraries for Python 3.8, 3.9 for ARM M1 MacBooks, the packages fail to install because Pythonn bindings compilation for M1 can only work for Python 3.10+.There is not an easy solution for that except commenting out graphviz dependency from setup.py, when you want to install Airflow for Python 3.8, 3.9 for MacBook M1.
However Graphviz is really used in two places:
when you want to render DAGs wia airflow CLI - either to an image or directly to terminal (for terminals/systems supporting imgcat)
when you want to rended ER diagram after you modified Airflow models
The latter is a development-only feature, the former is production feature, however it is a very niche one.
This PR turns rendering of the images in Airflow in optional feature (only working when graphviz python bindings are installed) and effectively turns graphviz into an optional extra (and removes it from requirements).
This is not a breaking chnage technically - the CLIs to reder the DAGs is still there and IF you already have graphviz installed, it will continue working as it did before. The only problem when it does not work is where you do not have graphviz installed it will raise an error and inform that you need it.
Graphviz will remain to be installed for most users:
The only change will be a new installation of new version of Airflow from the scratch, where graphviz wlll need to be specified as extra or installed separatley in order to enable DAG rendering option.
Taking into account this behaviour (which only requires to install a graphviz package), this should not be considered as a breaking change.
Extracted from: #36537
^ 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 newsfragments.