amoghrajesh commented on code in PR #69780:
URL: https://github.com/apache/airflow/pull/69780#discussion_r3636026541
##########
shared/secrets_backend/src/airflow_shared/secrets_backend/base.py:
##########
@@ -122,6 +122,12 @@ def _get_connection_class(self) -> type:
def _deserialize_connection_value(conn_class: type, conn_id: str, value:
str):
value = value.strip()
if value[0] == "{":
+ # JSON secrets stored by Airflow 2-era backends may lack both
"conn_type"
Review Comment:
```suggestion
# JSON secrets stored by Airflow 2 backends may lack both
"conn_type"
```
##########
shared/secrets_backend/src/airflow_shared/secrets_backend/base.py:
##########
@@ -122,6 +122,12 @@ def _get_connection_class(self) -> type:
def _deserialize_connection_value(conn_class: type, conn_id: str, value:
str):
value = value.strip()
if value[0] == "{":
+ # JSON secrets stored by Airflow 2-era backends may lack both
"conn_type"
+ # and "uri" (e.g. {"host": ..., "login": ..., "password": ...}).
This is
+ # valid: conn_type is intentionally optional on the SDK Connection
model
+ # for Airflow 2 -> 3 migration compatibility. Do not add
validation here
+ # that rejects such secrets; see
+ # https://github.com/apache/airflow/pull/61728
Review Comment:
```suggestion
# for Airflow 2 -> 3 migration compatibility.
# Check: https://github.com/apache/airflow/pull/61728
```
##########
shared/secrets_backend/tests/secrets_backend/test_base.py:
##########
@@ -124,6 +124,22 @@ def test_deserialize_connection_json(self,
sample_conn_json):
assert conn.conn_id == "test_conn"
assert conn._kwargs["conn_type"] == "mysql"
+ def test_deserialize_connection_json_without_conn_type(self):
+ """Airflow 2-era JSON secrets without conn_type or uri must keep
deserializing.
+
+ Guards the Airflow 2 -> 3 migration compatibility established in
+ https://github.com/apache/airflow/pull/61728.
+ """
Review Comment:
```suggestion
"""
Guards the Airflow 2 -> 3 migration compatibility established in
https://github.com/apache/airflow/pull/61728.
"""
```
##########
shared/secrets_backend/src/airflow_shared/secrets_backend/base.py:
##########
@@ -122,6 +122,12 @@ def _get_connection_class(self) -> type:
def _deserialize_connection_value(conn_class: type, conn_id: str, value:
str):
value = value.strip()
if value[0] == "{":
+ # JSON secrets stored by Airflow 2-era backends may lack both
"conn_type"
Review Comment:
Maybe also mention a TODO to remove it after min supported airflow version
in providers is 3.0
--
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]
