Skip to content

Add fallback region_name value to AWS Executors - #38704

Merged
potiuk merged 2 commits into
apache:mainfrom
Taragolis:aws-executors-fallback-region
Apr 3, 2024
Merged

potiuk merged 2 commits into
apache:mainfrom
Taragolis:aws-executors-fallback-region

Conversation

@Taragolis

Copy link
Copy Markdown
Contributor

Add fallback to region_name it might not obtained if configuration not persists in airflow.cfg or in env vars or configuration loaded without provider configurations

In addition use conf_vars helper for configure tests, it will automatically undo configuration in the end of test, so it reduce side effect when one test allow to pass/fail next text tests.

Originally it cause flakey behaviour in parallel non-db tests when it might run tests agains not properly configured tests in main or PRs

________________ TestAwsBatchExecutor.test_health_check_failure ________________
[gw6] linux -- Python 3.12.2 /usr/local/bin/python

self = <tests.providers.amazon.aws.executors.batch.test_batch_executor.TestAwsBatchExecutor object at 0x7effd436bc20>
mock_executor = <MagicMock name='load' id='139637296591360'>

    @mock.patch(
        "airflow.providers.amazon.aws.executors.batch.boto_schema.BatchDescribeJobsResponseSchema.load"
    )
    def test_health_check_failure(self, mock_executor):
        mock_executor.batch.describe_jobs.side_effect = Exception("Test_failure")
>       executor = AwsBatchExecutor()

tests/providers/amazon/aws/executors/batch/test_batch_executor.py:519: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
airflow/providers/amazon/aws/executors/batch/batch_executor.py:102: in __init__
    self.load_batch_connection(check_connection=False)
airflow/providers/amazon/aws/executors/batch/batch_executor.py:156: in load_batch_connection
    region_name = conf.get(CONFIG_GROUP_NAME, AllBatchConfigKeys.REGION_NAME)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <airflow.configuration.AirflowConfigParser object at 0x7f0002c10500>
section = 'aws_batch_executor', key = 'region_name', suppress_warnings = False
_extra_stacklevel = 0, kwargs = {}, warning_emitted = False
option_description = {}, deprecated_section = None, deprecated_key = None

    def get(  # type: ignore[override,misc]
        self,
        section: str,
        key: str,
        suppress_warnings: bool = False,
        _extra_stacklevel: int = 0,
        **kwargs,
    ) -> str | None:
        section = section.lower()
        key = key.lower()
        warning_emitted = False
        deprecated_section: str | None
        deprecated_key: str | None
    
        option_description = self.configuration_description.get(section, {}).get(key, {})
        if option_description.get("deprecated"):
            deprecation_reason = option_description.get("deprecation_reason", "")
            warnings.warn(
                f"The '{key}' option in section {section} is deprecated. {deprecation_reason}",
                DeprecationWarning,
                stacklevel=2 + _extra_stacklevel,
            )
        # For when we rename whole sections
        if section in self.inversed_deprecated_sections:
            deprecated_section, deprecated_key = (section, key)
            section = self.inversed_deprecated_sections[section]
            if not self._suppress_future_warnings:
                warnings.warn(
                    f"The config section [{deprecated_section}] has been renamed to "
                    f"[{section}]. Please update your `conf.get*` call to use the new name",
                    FutureWarning,
                    stacklevel=2 + _extra_stacklevel,
                )
            # Don't warn about individual rename if the whole section is renamed
            warning_emitted = True
        elif (section, key) in self.inversed_deprecated_options:
            # Handle using deprecated section/key instead of the new section/key
            new_section, new_key = self.inversed_deprecated_options[(section, key)]
            if not self._suppress_future_warnings and not warning_emitted:
                warnings.warn(
                    f"section/key [{section}/{key}] has been deprecated, you should use"
                    f"[{new_section}/{new_key}] instead. Please update your `conf.get*` call to use the "
                    "new name",
                    FutureWarning,
                    stacklevel=2 + _extra_stacklevel,
                )
                warning_emitted = True
            deprecated_section, deprecated_key = section, key
            section, key = (new_section, new_key)
        elif section in self.deprecated_sections:
            # When accessing the new section name, make sure we check under the old config name
            deprecated_key = key
   deprecated_section = self.deprecated_sections[section][0]
        else:
            deprecated_section, deprecated_key, _ = self.deprecated_options.get(
                (section, key), (None, None, None)
            )
        # first check environment variables
        option = self._get_environment_variables(
            deprecated_key,
            deprecated_section,
            key,
            section,
            issue_warning=not warning_emitted,
            extra_stacklevel=_extra_stacklevel,
        )
        if option is not None:
            return option
    
        # ...then the config file
        option = self._get_option_from_config_file(
            deprecated_key,
            deprecated_section,
            key,
            kwargs,
            section,
            issue_warning=not warning_emitted,
            extra_stacklevel=_extra_stacklevel,
        )
        if option is not None:
            return option
    
        # ...then commands
        option = self._get_option_from_commands(
            deprecated_key,
            deprecated_section,
            key,
            section,
            issue_warning=not warning_emitted,
            extra_stacklevel=_extra_stacklevel,
        )
        if option is not None:
            return option
    
        # ...then from secret backends
        option = self._get_option_from_secrets(
            deprecated_key,
            deprecated_section,
            key,
            section,
            issue_warning=not warning_emitted,
            extra_stacklevel=_extra_stacklevel,
        )
        if option is not None:
            return option
    
        # ...then the default config
        if self.get_default_value(section, key) is not None or "fallback" in kwargs:
            return expand_env_var(self.get_default_value(section, key, **kwargs))
    
        if self.get_default_pre_2_7_value(section, key) is not None:
            # no expansion needed
            return self.get_default_pre_2_7_value(section, key, **kwargs)
    
        if not suppress_warnings:
            log.warning("section/key [%s/%s] not found in config", section, key)
    
>       raise AirflowConfigException(f"section/key [{section}/{key}] not found in config")
E       airflow.exceptions.AirflowConfigException: section/key [aws_batch_executor/region_name] not found in config

airflow/configuration.py:1052: AirflowConfigException
----------------------------- Captured stdout call -----------------------------
[2024-04-03T10:14:13.235+0000] {batch_executor.py:150} INFO - Loading Connection information
[2024-04-03T10:14:13.237+0000] {configuration.py:1050} WARNING - section/*** [aws_batch_executor/region_name] not found in config

FAILED tests/providers/amazon/aws/executors/batch/test_batch_executor.py::TestAwsBatchExecutor::test_health_check_failure - airflow.exceptions.AirflowConfigException: section/key [aws_batch_executor/region_name] not found in config

^ 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.

@boring-cyborg boring-cyborg Bot added area:providers provider:amazon AWS/Amazon - related issues labels Apr 3, 2024
@Taragolis
Taragolis force-pushed the aws-executors-fallback-region branch from 40ec80c to 43d616e Compare April 3, 2024 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers provider:amazon AWS/Amazon - related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants