pankajkoti commented on code in PR #27264:
URL: https://github.com/apache/airflow/pull/27264#discussion_r1197527810
##########
CI.rst:
##########
@@ -59,7 +59,7 @@ Container Registry used as cache
We are using GitHub Container Registry to store the results of the ``Build
Images``
workflow which is used in the ``Tests`` workflow.
-Currently in main version of Airflow we run tests in 4 different versions of
Python (3.7, 3.8, 3.9, 3.10)
+Currently in main version of Airflow we run tests in those versions of Python
(3.7, 3.8, 3.9, 3.10, 3.11)
which means that we have to build 8 images (4 CI ones and 4 PROD ones). Yet we
run around 12 jobs
Review Comment:
do we need to update the numbers here, now? yes, perhaps we need to update
it back to the same numbers when we remove 3.7, but since we still have 3.7 in
this PR 🤔
Or maybe generalise this statement like we did with `those`, "that we have
to build 2x images (one for CI and one for Prod for each of those versions)"
##########
pyproject.toml:
##########
@@ -134,7 +134,26 @@ known-third-party = [
[tool.ruff.per-file-ignores]
"airflow/models/__init__.py" = ["F401"]
"airflow/models/sqla_models.py" = ["F401"]
-
+"tests/providers/google/cloud/_internal_client/test_secret_manager_client.py"
= ["E402"]
Review Comment:
Just curious, did these errors start coming only for 3.11?
##########
scripts/ci/pre_commit/pre_commit_update_providers_dependencies.py:
##########
@@ -187,7 +187,7 @@ def check_if_different_provider_used(file_path: Path) ->
None:
check_if_different_provider_used(file)
for provider, provider_yaml_content in ALL_PROVIDERS.items():
if not provider_yaml_content.get("suspended"):
-
ALL_DEPENDENCIES[provider][DEPS].extend(provider_yaml_content["dependencies"])
+
ALL_DEPENDENCIES[provider]["deps"].extend(provider_yaml_content["dependencies"])
Review Comment:
same question to understand the reasoning for this change.
##########
scripts/ci/pre_commit/pre_commit_update_providers_dependencies.py:
##########
@@ -175,7 +175,7 @@ def check_if_different_provider_used(file_path: Path) ->
None:
if imported_provider is not None and imported_provider not in
ALL_PROVIDERS:
warnings.append(f"The provider {imported_provider} from
{file_path} cannot be found.")
elif imported_provider and file_provider != imported_provider:
-
ALL_DEPENDENCIES[file_provider][CROSS_PROVIDERS_DEPS].append(imported_provider)
+
ALL_DEPENDENCIES[file_provider]["cross-providers-deps"].append(imported_provider)
Review Comment:
Is there a reason we're making this change?
--
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]
