Make dbapi inherit get_uri from connection

[bolkedebruin]

DBApi has its own get_uri method which does not deal
with quoting properly and neither with empty passwords.
Connection also has a get_uri method that deals properly
with the above issues.

@dimberman @ashb

[github-actions]

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

[potiuk]

NICE!.

BTW. I looked it up - the get_uri method was added (and with plenty of tests) in Airflow 2.0.0 and already present in the 1.0 verson of all providers so it shuld be backwards-compatible in all directions. I wonder why we have not updated it then :)

[bolkedebruin]

@potiuk which one did you look up? the one in dbapi was pretty buggy and I am not sure why it worked :-). The one in connection is a bit better, but I am confused why we use so little standardization here while url encoding is actually pretty hard.

[ashb]

I have a feeling Connection.get_uri is a "relatively" recent addition, I think added for airflow connections export command.

[potiuk]

I have a feeling Connection.get_uri is a "relatively" recent addition, I think added for airflow connections export command.

Yep. It is. And I think this is simply a very straightforward manifestation of "If is is not used, it likely does not work".

From what I looked today the "Connection" get_uri() was added in 2.0.0a.
And if you look closely - there is no "REAL" usage of get_uri even in DBApi. There is nearly no use case for it - except displaying it in the airflow CLI and possibly UI (which does not matter at all if it is correct or not).

The only actual usage of get_uri - from what I see - is "get_sqlalchemy_engine" which is actually hardly useful anyway. Getting sqlalchemy engine for a connection makes pretty much no sense. SQLAlchemy is really an ORM mapper, and when you have generic connection from "Connection" object - you have no objects schema - by definition - and SQLAlchemy is well, useless.

You can instantiate an SQLAlchemy session and run a raw query - maybe - but then you can run query via DBApi's "run" method anyway - so it makes no sense to instantiate the sqlalchemy engine for it.

So while we likely should keep backwards compatibility (just in case) with even "more correct" URIs returned, I think hardly anyone uses it or make some assumptions about returned URI correctness.

[bolkedebruin]

Interesting! Maybe we should move away from "conn_type" etc and just rely on people specifying a URI (as happens in others systems) and allow for specifying a key or username/password?

And btw @dimberman is relying on this function in the new sql-decorator (to be renamed I guess).

[potiuk]

Interesting! Maybe we should move away from "conn_type" etc and just rely on people specifying a URI (as happens in others systems) and allow for specifying a key or username/password?

I think connections need redesign badly. There are a number of ambiguities and duplications there (especially in-connection with extras and UI). And I also think @dimberman you already take steps in that direction too right ? :D.