Skip to content

Fix connection recovery from uri when host has protocol#51953

Merged
amoghrajesh merged 1 commit intoapache:mainfrom
astronomer:uri-handling-connections
Jun 20, 2025
Merged

Fix connection recovery from uri when host has protocol#51953
amoghrajesh merged 1 commit intoapache:mainfrom
astronomer:uri-handling-connections

Conversation

@amoghrajesh
Copy link
Contributor

@amoghrajesh amoghrajesh commented Jun 20, 2025

While working on #51930, realised that connections cannot recover using the uri of the same connection when hosts have protocols.

Example:

from airflow.models import Connection
Connection(conn_id="default_port", conn_type="http", host="http://host/")
Out[13]: default_port
x = Connection(conn_id="default_port", conn_type="http", host="http://host/")
x
Out[15]: default_port
x.host
Out[16]: 'http://host/'
x.as_json()
Out[17]: '{"conn_type": "http", "host": "http://host/"}'
x.get_uri()
Out[18]: 'http://host/'
y = Connection(uri=x.get_uri())
y.host
Out[20]: 'host'

This should not be the case, information should be preserved (as_json does it)

After fix:

from airflow.models import Connection
x = Connection(conn_id="default_port", conn_type="http", host="http://host")
x.as_json()
Out[5]: '{"conn_type": "http", "host": "http://host"}'
x.host
Out[6]: 'http://host'
y = Connection(uri=x.get_uri())
y.host
Out[8]: 'http://host'

We have enough test coverage in test_get_uri and related methods.

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@amoghrajesh amoghrajesh added this to the Airflow 3.0.3 milestone Jun 20, 2025
@amoghrajesh amoghrajesh self-assigned this Jun 20, 2025
@amoghrajesh amoghrajesh added the backport-to-v3-1-test Mark PR with this label to backport to v3-1-test branch label Jun 20, 2025
@amoghrajesh amoghrajesh mentioned this pull request Jun 20, 2025
Merged
5 tasks
@amoghrajesh amoghrajesh removed this from the Airflow 3.0.3 milestone Jun 20, 2025
@amoghrajesh amoghrajesh removed the backport-to-v3-1-test Mark PR with this label to backport to v3-1-test branch label Jun 20, 2025
@potiuk
Copy link
Member

potiuk commented Jun 20, 2025

Nice one!

ashb
ashb reviewed Jun 20, 2025
View reviewed changes
@amoghrajesh amoghrajesh requested a review from ashb June 20, 2025 10:12
@amoghrajesh
Copy link
Contributor Author

amoghrajesh commented Jun 20, 2025

@ashb I am merging this. Let me know if you are unhappy with the connection type. I will fix it

@amoghrajesh amoghrajesh merged commit 2d6d594 into apache:main Jun 20, 2025
76 checks passed
@amoghrajesh amoghrajesh deleted the uri-handling-connections branch June 20, 2025 11:06
@amoghrajesh amoghrajesh mentioned this pull request Jun 20, 2025
Merged
RoyLee1224 pushed a commit to RoyLee1224/airflow that referenced this pull request Jun 21, 2025
@amoghrajesh @RoyLee1224
Fix connection recovery from uri when host has protocol (apache#51953)
612bc3a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@potiuk potiuk potiuk approved these changes

@kaxil kaxil Awaiting requested review from kaxil kaxil is a code owner

@XD-DENG XD-DENG Awaiting requested review from XD-DENG XD-DENG is a code owner

@hussein-awala hussein-awala Awaiting requested review from hussein-awala

@vatsrahul1001 vatsrahul1001 Awaiting requested review from vatsrahul1001

@ashb ashb Awaiting requested review from ashb ashb is a code owner

Assignees

@amoghrajesh amoghrajesh

Labels

area:task-sdk

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@amoghrajesh @potiuk @ashb