Zeeshan-Chaudhry commented on code in PR #73262: URL: https://github.com/apache/airflow/pull/73262#discussion_r4033205491
########## providers/common/compat/tests/unit/common/compat/standard/test_operators.py: ########## @@ -0,0 +1,95 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import functools + +import pytest + +from airflow.providers.common.compat.standard import operators + +from tests_common.test_utils.version_compat import AIRFLOW_V_3_2_PLUS + +EXPECTED_EXPORTS = ( + "BaseAsyncOperator", + "BaseBranchOperator", + "BaseOperator", + "BranchMixIn", + "PythonOperator", + "ShortCircuitOperator", + "_SERIALIZERS", + "get_current_context", + "is_async_callable", +) + + +async def async_function(): + """Sample coroutine function.""" + + +def sync_function(): + """Sample plain function.""" + + +def test_public_exports(): + assert set(operators.__all__) == set(EXPECTED_EXPORTS) + + [email protected]("name", EXPECTED_EXPORTS) +def test_all_compat_imports_work(name): Review Comment: Done in 6943782. Dropped both keys, so every remaining `_IMPORT_MAP` entry routes through `create_module_getattr` and the resolve-each-export test now exercises it. One deviation from your suggestion: a literal `__all__` list trips ruff `F822` on the seven lazily resolved names, and there is no `noqa: F822` precedent in the repo, so I kept it computed in the style of `sdk.py`: ```python __all__ = sorted([*_IMPORT_MAP, "BaseAsyncOperator", "is_async_callable"]) ``` `operators.__all__` is byte for byte what it was, both names still import directly, and `__getattr__` now raises the plain `module has no attribute` for them instead of an `ImportError`. Happy to switch to a literal list with a noqa if you prefer that. ########## providers/common/compat/tests/unit/common/compat/standard/test_operators.py: ########## @@ -0,0 +1,95 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import functools + +import pytest + +from airflow.providers.common.compat.standard import operators + +from tests_common.test_utils.version_compat import AIRFLOW_V_3_2_PLUS + +EXPECTED_EXPORTS = ( + "BaseAsyncOperator", + "BaseBranchOperator", + "BaseOperator", + "BranchMixIn", + "PythonOperator", + "ShortCircuitOperator", + "_SERIALIZERS", + "get_current_context", + "is_async_callable", +) + + +async def async_function(): + """Sample coroutine function.""" + + +def sync_function(): + """Sample plain function.""" + + +def test_public_exports(): + assert set(operators.__all__) == set(EXPECTED_EXPORTS) + + [email protected]("name", EXPECTED_EXPORTS) +def test_all_compat_imports_work(name): + assert getattr(operators, name) is not None + + +def test_invalid_import_raises_attribute_error(): Review Comment: Dropped in both files in 6943782. ########## providers/common/compat/tests/unit/common/compat/standard/test_triggers.py: ########## @@ -0,0 +1,37 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import pytest + +from airflow.providers.common.compat.standard import triggers + +EXPECTED_EXPORTS = ("TimeDeltaTrigger",) + + +def test_public_exports(): + assert set(triggers.__all__) == set(EXPECTED_EXPORTS) + + [email protected]("name", EXPECTED_EXPORTS) +def test_all_compat_imports_work(name): + assert getattr(triggers, name) is not None + + +def test_invalid_import_raises_attribute_error(): Review Comment: Dropped here too in 6943782. ########## providers/common/compat/tests/unit/common/compat/standard/test_operators.py: ########## @@ -0,0 +1,95 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import functools + +import pytest + +from airflow.providers.common.compat.standard import operators + +from tests_common.test_utils.version_compat import AIRFLOW_V_3_2_PLUS + +EXPECTED_EXPORTS = ( + "BaseAsyncOperator", + "BaseBranchOperator", + "BaseOperator", + "BranchMixIn", + "PythonOperator", + "ShortCircuitOperator", + "_SERIALIZERS", + "get_current_context", + "is_async_callable", +) + + +async def async_function(): + """Sample coroutine function.""" + + +def sync_function(): + """Sample plain function.""" + + +def test_public_exports(): + assert set(operators.__all__) == set(EXPECTED_EXPORTS) + + [email protected]("name", EXPECTED_EXPORTS) +def test_all_compat_imports_work(name): + assert getattr(operators, name) is not None + + +def test_invalid_import_raises_attribute_error(): + with pytest.raises(AttributeError, match="module has no attribute 'NonExistentClass'"): + _ = operators.NonExistentClass + + [email protected]( + ("func", "expected"), + [ + pytest.param(async_function, True, id="coroutine-function"), + pytest.param(sync_function, False, id="plain-function"), + pytest.param(functools.partial(async_function), True, id="partial-of-coroutine-function"), + pytest.param(functools.partial(sync_function), False, id="partial-of-plain-function"), + pytest.param( + functools.partial(functools.partial(async_function)), + True, + id="nested-partial-of-coroutine-function", + ), + ], +) +def test_is_async_callable(func, expected): + """ + Coroutine functions are detected through any number of ``functools.partial`` wrappers. + + These cases hold on both sides of the Airflow 3.2 fork: the local stub unwraps partials in a + loop, and the real implementation does the same through ``unwrap_callable``. + """ + assert operators.is_async_callable(func) is expected + + [email protected](AIRFLOW_V_3_2_PLUS, reason="The BaseAsyncOperator stub only exists on Airflow < 3.2") +class TestBaseAsyncOperatorStub: + def test_is_async(self): Review Comment: Kept as is. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
