From 4755fe40350c83fafef4525bf47040e1a1f0f07a Mon Sep 17 00:00:00 2001 From: "D. Ferruzzi" Date: Fri, 11 Aug 2023 13:15:01 -0700 Subject: [PATCH] D205 Support - Root files (#33297) * D205 Support - Root files Updates setup.py, airflow/configuration.py, and airflow/exceptions.py * missed one --- airflow/configuration.py | 24 +++++++++++------ airflow/exceptions.py | 5 +--- setup.py | 58 +++++++++++++++++++++++++--------------- 3 files changed, 53 insertions(+), 34 deletions(-) diff --git a/airflow/configuration.py b/airflow/configuration.py index 57e8e5087d9e..8cdc2a1542d4 100644 --- a/airflow/configuration.py +++ b/airflow/configuration.py @@ -240,6 +240,7 @@ def is_template(self, section: str, key) -> bool: def _update_defaults_from_string(self, config_string: str): """ The defaults in _default_values are updated based on values in config_string ("ini" format). + Note that those values are not validated and cannot contain variables because we are using regular config parser to load them. This method is used to test the config parser in unit tests. @@ -483,8 +484,9 @@ def get_sections_including_defaults(self) -> list[str]: def get_options_including_defaults(self, section: str) -> list[str]: """ - Retrieves all possible option from the configuration parser for the section given, - including options defined by built-in defaults. + Retrieves all possible option from the configuration parser for the section given. + + Includes options defined by built-in defaults. :return: list of option names for the section given """ @@ -500,6 +502,7 @@ def get_options_including_defaults(self, section: str) -> list[str]: def optionxform(self, optionstr: str) -> str: """ This method transforms option names on every read, get, or set operation. + This changes from the default behaviour of ConfigParser from lower-casing to instead be case-preserving. @@ -511,8 +514,10 @@ def optionxform(self, optionstr: str) -> str: @contextmanager def make_sure_configuration_loaded(self, with_providers: bool) -> Generator[None, None, None]: """ - Make sure configuration is loaded with or without providers, regardless if the provider configuration - has been loaded before or not. Restores configuration to the state before entering the context. + Make sure configuration is loaded with or without providers. + + This happens regardless if the provider configuration has been loaded before or not. + Restores configuration to the state before entering the context. :param with_providers: whether providers should be loaded """ @@ -740,6 +745,7 @@ def validate(self): def _validate_max_tis_per_query(self) -> None: """ Check if config ``scheduler.max_tis_per_query`` is not greater than ``core.parallelism``. + If not met, a warning message is printed to guide the user to correct it. More info: https://github.com/apache/airflow/pull/32572 @@ -1952,10 +1958,12 @@ def create_default_config_parser(configuration_description: dict[str, dict[str, def create_pre_2_7_defaults() -> ConfigParser: """ - Creates parser using the old defaults from Airflow < 2.7.0, in order to be able to fall-back to those - defaults when old version of provider, not supporting "config contribution" is installed with Airflow - 2.7.0+. This "default" configuration does not support variable expansion, those are pretty much - hard-coded defaults we want to fall-back to in such case. + Creates parser using the old defaults from Airflow < 2.7.0. + + This is used in order to be able to fall-back to those defaults when old version of provider, + not supporting "config contribution" is installed with Airflow 2.7.0+. This "default" + configuration does not support variable expansion, those are pretty much hard-coded defaults ' + we want to fall-back to in such case. """ config_parser = ConfigParser() config_parser.read(_default_config_file_path("pre_2_7_defaults.cfg")) diff --git a/airflow/exceptions.py b/airflow/exceptions.py index fe0c4e416bcd..b471297cd959 100644 --- a/airflow/exceptions.py +++ b/airflow/exceptions.py @@ -176,10 +176,7 @@ class AirflowClusterPolicySkipDag(AirflowException): class AirflowClusterPolicyError(AirflowException): - """ - Raise when there is an error in Cluster Policy, - except AirflowClusterPolicyViolation and AirflowClusterPolicySkipDag. - """ + """Raise for a Cluster Policy other than AirflowClusterPolicyViolation or AirflowClusterPolicySkipDag.""" class AirflowTimetableInvalid(AirflowException): diff --git a/setup.py b/setup.py index 6f29dc1f4705..d0302981c5ff 100644 --- a/setup.py +++ b/setup.py @@ -63,6 +63,8 @@ def apply_pypi_suffix_to_airflow_packages(dependencies: list[str]) -> None: """ + Apply version suffix to dependencies that do not have one. + Looks through the list of dependencies, finds which one are airflow or airflow providers packages and applies the version suffix to those of them that do not have the suffix applied yet. @@ -130,6 +132,7 @@ def airflow_test_suite() -> unittest.TestSuite: class CleanCommand(Command): """ Command to tidy up the project root. + Registered as cmdclass in setup() so it can be called with ``python setup.py extra_clean``. """ @@ -166,6 +169,7 @@ def run(self) -> None: class CompileAssets(Command): """ Compile and build the frontend assets using yarn and webpack. + Registered as cmdclass in setup() so it can be called with ``python setup.py compile_assets``. """ @@ -187,7 +191,8 @@ def run(self) -> None: class ListExtras(Command): """ - List all available extras + List all available extras. + Registered as cmdclass in setup() so it can be called with ``python setup.py list_extras``. """ @@ -207,11 +212,12 @@ def run(self) -> None: def git_version() -> str: """ - Return a version to identify the state of the underlying git repo. The version will - indicate whether the head of the current git-backed working directory is tied to a - release tag or not : it will indicate the former with a 'release:{version}' prefix - and the latter with a '.dev0' suffix. Following the prefix will be a sha of the current - branch head. Finally, a "dirty" suffix is appended to indicate that uncommitted + Return a version to identify the state of the underlying git repo. + + The version will indicate whether the head of the current git-backed working directory + is tied to a release tag or not : it will indicate the former with a 'release:{version}' + prefix and the latter with a '.dev0' suffix. Following the prefix will be a sha of the + current branch head. Finally, a "dirty" suffix is appended to indicate that uncommitted changes are present. :return: Found Airflow version in Git repo @@ -596,8 +602,9 @@ def add_additional_extras() -> None: def add_extras_for_all_deprecated_aliases() -> None: """ - Add extras for all deprecated aliases. Requirements for those deprecated aliases are the same - as the extras they are replaced with. + Add extras for all deprecated aliases. + + Requirements for those deprecated aliases are the same as the extras they are replaced with. The dependencies are not copies - those are the same lists as for the new extras. This is intended. Thanks to that if the original extras are later extended with providers, aliases are extended as well. """ @@ -610,8 +617,7 @@ def add_extras_for_all_deprecated_aliases() -> None: def add_all_deprecated_provider_packages() -> None: """ - For deprecated aliases that are providers, we will swap the providers dependencies to instead - be the provider itself. + For deprecated aliases that are providers, swap the providers dependencies to be the provider itself. e.g. {"kubernetes": ["kubernetes>=3.0.0, <12.0.0", ...]} becomes {"kubernetes": ["apache-airflow-provider-cncf-kubernetes"]} @@ -744,6 +750,7 @@ def remove_provider_limits(package: str) -> str: def sort_extras_dependencies() -> dict[str, list[str]]: """ The dictionary order remains when keys() are retrieved. + Sort both: extras and list of dependencies to make it easier to analyse problems external packages will be first, then if providers are added they are added at the end of the lists. """ @@ -812,8 +819,8 @@ def __init__(self, attrs=None): def parse_config_files(self, *args, **kwargs) -> None: """ - Ensure that when we have been asked to install providers from sources - that we don't *also* try to install those providers from PyPI. + When asked to install providers from sources, ensure we don't *also* try to install from PyPI. + Also we should make sure that in this case we copy provider.yaml files so that Providers manager can find package information. """ @@ -837,11 +844,13 @@ def parse_config_files(self, *args, **kwargs) -> None: def replace_extra_dependencies_with_provider_packages(extra: str, providers: list[str]) -> None: """ - Replaces extra dependencies with provider package. The intention here is that when - the provider is added as dependency of extra, there is no need to add the dependencies - separately. This is not needed and even harmful, because in case of future versions of - the provider, the dependencies might change, so hard-coding dependencies from the version - that was available at the release time might cause dependency conflicts in the future. + Replaces extra dependencies with provider package. + + The intention here is that when the provider is added as dependency of extra, there is no + need to add the dependencies separately. This is not needed and even harmful, because in + case of future versions of the provider, the dependencies might change, so hard-coding + dependencies from the version that was available at the release time might cause dependency + conflicts in the future. Say for example that you have salesforce provider with those deps: @@ -888,9 +897,11 @@ def replace_extra_dependencies_with_provider_packages(extra: str, providers: lis def add_provider_packages_to_extra_dependencies(extra: str, providers: list[str]) -> None: """ - Adds provider packages as dependencies to extra. This is used to add provider packages as dependencies - to the "bulk" kind of extras. Those bulk extras do not have the detailed 'extra' dependencies as - initial values, so instead of replacing them (see previous function) we can extend them. + Adds provider packages as dependencies to extra. + + This is used to add provider packages as dependencies to the "bulk" kind of extras. + Those bulk extras do not have the detailed 'extra' dependencies as initial values, + so instead of replacing them (see previous function) we can extend them. :param extra: Name of the extra to add providers to :param providers: list of provider ids @@ -902,6 +913,8 @@ def add_provider_packages_to_extra_dependencies(extra: str, providers: list[str] def add_all_provider_packages() -> None: """ + Add extra dependencies when providers are installed from packages. + In case of regular installation (providers installed from packages), we should add extra dependencies to Airflow - to get the providers automatically installed when those extras are installed. @@ -965,8 +978,9 @@ def do_setup() -> None: def include_provider_namespace_packages_when_installing_from_sources() -> None: """ - When installing providers from sources we install all namespace packages found below airflow, - including airflow and provider packages, otherwise defaults from setup.cfg control this. + When installing providers from sources we install all namespace packages found below airflow. + + Includes airflow and provider packages, otherwise defaults from setup.cfg control this. The kwargs in setup() call override those that are specified in setup.cfg. """ if os.getenv(INSTALL_PROVIDERS_FROM_SOURCES) == "true":