amoghrajesh commented on code in PR #46891:
URL: https://github.com/apache/airflow/pull/46891#discussion_r2209289426


##########
.pre-commit-config.yaml:
##########
@@ -1014,7 +1014,12 @@ repos:
         name: Update Airflow's meta-package pyproject.toml
         language: python
         entry: ./scripts/ci/pre_commit/update_airflow_pyproject_toml.py
-        files: 
^.*/pyproject\.toml$|^scripts/ci/pre_commit/update_airflow_pyproject_toml\.py$
+        files: >
+          (?x)
+          ^.*/pyproject\.toml$|
+          ^scripts/ci/pre_commit/update_airflow_pyproject_toml\.py$|
+          ^providers/.*/pyproject\.toml$|
+          ^providers/.*/provider\.yaml$

Review Comment:
   This wasn't present earlier?



##########
providers/apache/beam/pyproject.toml:
##########
@@ -57,20 +57,21 @@ requires-python = ">=3.10"
 # After you modify the dependencies, and rebuild your Breeze CI image with 
``breeze ci-image build``
 dependencies = [
     "apache-airflow>=2.10.0",
-    'apache-beam>=2.60.0',
-    "pyarrow>=16.1.0",
-    "numpy>=1.26.0",
-
+    'apache-beam>=2.60.0; python_version < "3.13"',
+    "pyarrow>=16.1.0; python_version < '3.13'",
+    "numpy>=1.22.4; python_version<'3.11'",
+    "numpy>=1.23.2; python_version<'3.12' and python_version>='3.11'",
+    "numpy>=1.26.0; python_version>='3.12' and python_version < '3.13'",

Review Comment:
   Should we be adding for 3.13 too?



##########
scripts/ci/pre_commit/update_airflow_pyproject_toml.py:
##########
@@ -132,6 +141,23 @@ def find_min_provider_version(provider_id: str) -> 
tuple[Version | None, str]:
 
 PROVIDER_MIN_VERSIONS: dict[str, str | None] = {}
 
+
+def get_python_exclusion(provider_dependencies: dict[str, Any]) -> str:
+    """
+    Get the python exclusion for the provider based on its metadata.
+    If the provider is not in the metadata, return an empty string.
+    """

Review Comment:
   nit:
   ```
   """
       Return a Python version exclusion marker string based on provider 
metadata.
   
       If there are excluded Python versions in the metadata, this function 
returns a
       marker string like: '; python_version != "3.8" and python_version != 
"3.11"'
   
       If none are found, it returns an empty str.
       """
   ```



##########
providers/amazon/pyproject.toml:
##########
@@ -95,9 +96,12 @@ dependencies = [
     "s3fs>=2023.10.0",
 ]
 "python3-saml" = [
-    "python3-saml>=1.16.0",
-    "xmlsec>=1.3.14",
-    "lxml>=6.0.0",
+    # Python 3 saml is not compatible with Python 3.13 yet, so we pin it to < 
3.13
+    "python3-saml>=1.16.0; python_version < \"3.13\"",
+    # python3-saml is dependent on xmlsec and seems they do not pin it, 
pinning here would be needed
+    # We can remove it after 
https://github.com/xmlsec/python-xmlsec/issues/344 is fixed
+    "xmlsec>=1.3.14; python_version < \"3.13\"",
+    "lxml>=6.0.0; python_version < \"3.13\"",

Review Comment:
   I see in a couple of places its escaped and in some, not. If possible, can 
we make it uniform?



##########
airflow-core/pyproject.toml:
##########
@@ -133,9 +133,7 @@ dependencies = [
     "tabulate>=0.9.0",
     "tenacity>=8.3.0",
     "termcolor>=3.0.0",
-    # temporarily exclude 4.14.0 due to its broken compat with cadwyn
-    # See https://github.com/zmievsa/cadwyn/issues/283
-    "typing-extensions!=4.14.0",
+    "typing-extensions>=4.14.1",

Review Comment:
   Sounds good!



##########
dev/breeze/tests/test_selective_checks.py:
##########
@@ -1236,7 +1236,15 @@ def test_excluded_providers():
         {
             "excluded-providers-as-string": json.dumps(
                 {
+<<<<<<< HEAD
                     "3.13": ["apache.beam", "apache.kafka", "fab", "ydb"],
+||||||| parent of d7c0418248 (Pyproject.toml provider changes for Python 3.13)
+                    "3.9": ["cloudant"],
+                    "3.13": ["apache.beam", "apache.kafka", "fab", "ydb"],
+=======
+                    "3.9": ["cloudant"],
+                    "3.13": ["apache.beam", "apache.kafka", "edge3", "fab", 
"yandex", "ydb"],
+>>>>>>> d7c0418248 (Pyproject.toml provider changes for Python 3.13)

Review Comment:
   Worth double checking if this is resolved with later commits



##########
providers/amazon/pyproject.toml:
##########
@@ -95,9 +96,12 @@ dependencies = [
     "s3fs>=2023.10.0",
 ]
 "python3-saml" = [
-    "python3-saml>=1.16.0",
-    "xmlsec>=1.3.14",
-    "lxml>=6.0.0",
+    # Python 3 saml is not compatible with Python 3.13 yet, so we pin it to < 
3.13
+    "python3-saml>=1.16.0; python_version < \"3.13\"",
+    # python3-saml is dependent on xmlsec and seems they do not pin it, 
pinning here would be needed
+    # We can remove it after 
https://github.com/xmlsec/python-xmlsec/issues/344 is fixed
+    "xmlsec>=1.3.14; python_version < \"3.13\"",
+    "lxml>=6.0.0; python_version < \"3.13\"",

Review Comment:
   Do we need to escape these? Can't we just do: `"tomli==2.2.1; python_version 
< '3.11'",` for example?
   



-- 
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]

Reply via email to