Skip to content

Conversation

@Pyasma
Copy link
Contributor

@Pyasma Pyasma commented Dec 11, 2025

RE:This PR fixes how Connection.get_uri() handles extras with non-string values (floats, booleans, nested dicts).

Extras are now stringified before encoding to produce valid, SQLAlchemy-compatible URIs.

Added a unit test test_get_uri_with_non_string_extras to verify the change.

All pre-commit checks passed.

Issue :#55893

Original PR is closed -> #57969


^ 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.

@Pyasma Pyasma requested review from XD-DENG and ashb as code owners December 11, 2025 20:29
@Pyasma Pyasma changed the title Fix/connection uri Fix/RE:Invalid uri created when extras contains non string elements Dec 11, 2025
@Pyasma
Copy link
Contributor Author

Pyasma commented Dec 12, 2025

CodeQL workflow is awaiting maintainer approval (standard for PRs from forks). All unit tests pass locally.
@jason810496
@Lee-W

@Pyasma
Copy link
Contributor Author

Pyasma commented Dec 12, 2025

CodeQL workflow is awaiting maintainer approval (standard for PRs from forks). All unit tests pass locally. @jason810496 @Lee-W

Can you review this please

@Pyasma
Copy link
Contributor Author

Pyasma commented Dec 14, 2025

Hey @potiuk, thanks for re-running the tests. I’m a bit confused by these errors—do you have any advice on where I should start looking or how to approach fixing them?

@potiuk potiuk force-pushed the Fix/ConnectionURI branch 2 times, most recently from 96581bd to 6e6b804 Compare December 14, 2025 07:47
@potiuk
Copy link
Member

potiuk commented Dec 14, 2025

Hey @potiuk, thanks for re-running the tests. I’m a bit confused by these errors—do you have any advice on where I should start looking or how to approach fixing them?

There was a flaky test in main - so it failed often in other PRs as well. This is now hopefully fixed in #59394 so you should not worry about it.

@henry3260
Copy link
Contributor

Same 🫠

@Pyasma
Copy link
Contributor Author

Pyasma commented Dec 14, 2025

test still failed though 🥲
I will look at it later any advice for this one ?

@potiuk
Copy link
Member

potiuk commented Dec 14, 2025

Yep. the hopefully part was unfortunatelly well placed.

@henry3260
Copy link
Contributor

test still failed though 🥲

I will look at it later any advice for this one ?

Don't worry. It's not you fault

@potiuk
Copy link
Member

potiuk commented Dec 14, 2025

Yeah. Not your fault for sure. I have a likely fix for the main issue: #59406

@Pyasma
Copy link
Contributor Author

Pyasma commented Dec 15, 2025

Nice all test passed thanks @potiuk @henry3260

The issue is solved now?

@henry3260
Copy link
Contributor

Nice all test passed thanks @potiuk @henry3260

The issue is solved now?

Yes 😀

@Pyasma
Copy link
Contributor Author

Pyasma commented Dec 16, 2025

@jason810496 Hey jason can you please review the PR

@Pyasma
Copy link
Contributor Author

Pyasma commented Dec 16, 2025

hey @Lee-W Can you please review the PR

Copy link
Member

@jason810496 jason810496 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update!

Copy link
Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one nit, overall looks good

@Pyasma
Copy link
Contributor Author

Pyasma commented Dec 22, 2025

sorry about this push i am gonna push one more time i forgot to do prek test

@Pyasma
Copy link
Contributor Author

Pyasma commented Dec 27, 2025

@jason810496 @Lee-W Thanks for approving the PR can you merge this to main?

@Lee-W Lee-W merged commit a377033 into apache:main Dec 27, 2025
69 checks passed
@jason810496
Copy link
Member

Sorry that I forgot to add the backport label. @Pyasma, would you mind backporting this PR to the v3-1-test branch? Thanks!

We can backport the patch using the following command and resolve any conflicts if needed:

cherry_picker a377033 v3-1-test

amoghrajesh pushed a commit to astronomer/airflow that referenced this pull request Dec 29, 2025
Subham-KRLX pushed a commit to Subham-KRLX/airflow that referenced this pull request Jan 2, 2026
@potiuk potiuk added this to the Airflow 3.1.6 milestone Jan 7, 2026
potiuk pushed a commit to potiuk/airflow that referenced this pull request Jan 7, 2026
…g elements (apache#59339)

(cherry picked from commit a377033)

Co-authored-by: Piyush Mudgal <94688939+Pyasma@users.noreply.github.com>
Co-authored-by: Pyasma <pranyasharms55555@gmail.com>
@potiuk
Copy link
Member

potiuk commented Jan 7, 2026

I cherry-picked it here #60219 @jason810496

@Pyasma
Copy link
Contributor Author

Pyasma commented Jan 7, 2026

hey @jason810496 Sorry i missed the message and thanks @potiuk

potiuk added a commit that referenced this pull request Jan 7, 2026
…g elements (#59339) (#60219)

(cherry picked from commit a377033)

Co-authored-by: Piyush Mudgal <94688939+Pyasma@users.noreply.github.com>
Co-authored-by: Pyasma <pranyasharms55555@gmail.com>
stegololz pushed a commit to stegololz/airflow that referenced this pull request Jan 9, 2026
@frodo2000
Copy link
Contributor

After migration to 3.1.6 still having error:

`[2026-01-13 15:46:29] ERROR - Task failed with exception source=task loc=task_runner.py:1056
AirflowException: ('Entra Id processing error:', TypeError("connect() got an unexpected keyword argument 'extra'"))
File "/home/airflow/airflow_venv/lib/python3.12/site-packages/airflow/sdk/execution_time/task_runner.py", line 1004 in run

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/airflow/sdk/execution_time/task_runner.py", line 1405 in _execute_task

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/airflow/sdk/bases/operator.py", line 417 in wrapper

File "/opt/airflow/plugins/com/softwaremind/transfers/entraid.py", line 148 in execute

TypeError: connect() got an unexpected keyword argument 'extra'
File "/opt/airflow/plugins/com/softwaremind/transfers/entraid.py", line 133 in execute

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/airflow/providers/microsoft/mssql/hooks/mssql.py", line 97 in get_sqlalchemy_connection

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/sqlalchemy/engine/base.py", line 3325 in connect

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/sqlalchemy/engine/base.py", line 96 in init

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/sqlalchemy/engine/base.py", line 3404 in raw_connection

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/sqlalchemy/engine/base.py", line 3371 in _wrap_pool_connect

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/sqlalchemy/pool/base.py", line 327 in connect

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/sqlalchemy/pool/base.py", line 894 in _checkout

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/sqlalchemy/pool/base.py", line 493 in checkout

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/sqlalchemy/pool/impl.py", line 145 in _do_get

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/sqlalchemy/util/langhelpers.py", line 70 in exit

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/sqlalchemy/util/compat.py", line 211 in raise_

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/sqlalchemy/pool/impl.py", line 143 in _do_get

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/sqlalchemy/pool/base.py", line 273 in _create_connection

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/sqlalchemy/pool/base.py", line 388 in init

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/sqlalchemy/pool/base.py", line 690 in __connect

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/sqlalchemy/util/langhelpers.py", line 70 in exit

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/sqlalchemy/util/compat.py", line 211 in raise_

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/sqlalchemy/pool/base.py", line 686 in __connect

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/sqlalchemy/engine/create.py", line 574 in connect

File "/home/airflow/airflow_venv/lib/python3.12/site-packages/sqlalchemy/engine/default.py", line 598 in connect`

When extra fields contains as_dict attribute:

{ "tds_version": "7.3", "as_dict": true }

@henry3260
Copy link
Contributor

henry3260 commented Jan 13, 2026

@frodo2000 Can you open an issue for that?
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants