amoghrajesh commented on code in PR #46891:
URL: https://github.com/apache/airflow/pull/46891#discussion_r2209317437
##########
airflow-core/src/airflow/api_fastapi/core_api/app.py:
##########
@@ -124,6 +127,13 @@ def init_flask_plugins(app: FastAPI) -> None:
try:
from airflow.providers.fab.www.app import create_app
except ImportError:
+ if PY313:
+ log.info(
Review Comment:
log.warning instead?
##########
airflow-core/tests/unit/always/test_example_dags.py:
##########
@@ -82,12 +82,7 @@ def get_suspended_providers_folders() -> list[str]:
for provider_path in AIRFLOW_PROVIDERS_ROOT_PATH.rglob("provider.yaml"):
provider_yaml = yaml.safe_load(provider_path.read_text())
if provider_yaml["state"] == "suspended":
- suspended_providers.append(
- provider_path.parent.relative_to(AIRFLOW_ROOT_PATH)
- .as_posix()
- # TODO(potiuk): check
- .replace("providers/src/airflow/providers/", "")
- )
+
suspended_providers.append(provider_path.parent.resolve().as_posix())
Review Comment:
Nice :)
##########
airflow-core/tests/unit/serialization/test_dag_serialization.py:
##########
@@ -1229,10 +1234,6 @@ def test_extra_serialized_field_and_operator_links(
link = simple_task.get_extra_links(ti, name)
assert link == expected
- # Test Deserialized link registered via Airflow Plugin
- link = simple_task.get_extra_links(ti, GoogleLink.name)
- assert link == "https://www.google.com";
Review Comment:
We would need to test this part as well right?
##########
task-sdk/pyproject.toml:
##########
@@ -21,7 +21,11 @@ dynamic = ["version"]
description = "Python Task SDK for Apache Airflow DAG Authors"
readme = { file = "README.md", content-type = "text/markdown" }
license-files.globs = ["LICENSE"]
-requires-python = ">=3.10"
+# We know that it will take a while before we can support Python 3.14 because
of all our dependencies
+# in all our providers - and usually it will take a while before we can
support it for majority of
+# providers, so we proactively limit it to <3.14.
+# It takes about 4-7 months after Python release before we can support it
+requires-python = ">=3.10, <3.14"
Review Comment:
SG
##########
airflow-core/pyproject.toml:
##########
@@ -35,7 +35,12 @@ name = "apache-airflow-core"
description = "Core packages for Apache Airflow, schedule and API server"
readme = { file = "README.md", content-type = "text/markdown" }
license-files.globs = ["LICENSE", "3rd-party-licenses/*.txt", "NOTICE"]
-requires-python = ">=3.10"
+# We know that it will take a while before we can support Python 3.14 because
of all our dependencies
+# It takes about 4-7 months after Python release before we can support it, so
we limit it to <3.14
+# proactively. This way we also have a chance to test it with Python 3.14 and
bunmp the upper binding
Review Comment:
nit: bunmp
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_dag_run.py:
##########
@@ -82,6 +88,7 @@
@pytest.fixture(autouse=True)
@provide_session
def setup(request, dag_maker, session=None):
+ clear_db_connections()
Review Comment:
Why was this needed? The tests dont really use connections directly
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_dags.py:
##########
@@ -58,6 +63,7 @@ class TestDagEndpoint:
@staticmethod
def _clear_db():
+ clear_db_connections()
Review Comment:
Same qn here
##########
airflow-core/tests/unit/cli/commands/test_api_server_command.py:
##########
@@ -296,16 +297,31 @@ def test_run_command_daemon(
)
]
mock_pid_file.assert_has_calls([mock.call(mock_setup_locations.return_value[0],
-1)])
- assert mock_open.mock_calls == [
- mock.call(mock_setup_locations.return_value[1], "a"),
- mock.call().__enter__(),
- mock.call(mock_setup_locations.return_value[2], "a"),
- mock.call().__enter__(),
- mock.call().truncate(0),
- mock.call().truncate(0),
- mock.call().__exit__(None, None, None),
- mock.call().__exit__(None, None, None),
- ]
+ if sys.version_info >= (3, 13):
+ # extra close is called in Python 3.13+ to close the file
descriptors
+ assert mock_open.mock_calls == [
+ mock.call(mock_setup_locations.return_value[1], "a"),
+ mock.call().__enter__(),
+ mock.call(mock_setup_locations.return_value[2], "a"),
+ mock.call().__enter__(),
+ mock.call().truncate(0),
+ mock.call().truncate(0),
+ mock.call().__exit__(None, None, None),
+ mock.call().close(),
+ mock.call().__exit__(None, None, None),
+ mock.call().close(),
+ ]
+ else:
+ assert mock_open.mock_calls == [
+ mock.call(mock_setup_locations.return_value[1], "a"),
+ mock.call().__enter__(),
+ mock.call(mock_setup_locations.return_value[2], "a"),
+ mock.call().__enter__(),
+ mock.call().truncate(0),
+ mock.call().truncate(0),
+ mock.call().__exit__(None, None, None),
+ mock.call().__exit__(None, None, None),
+ ]
Review Comment:
Weird that is not backward compatible!
--
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: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
