> Recently, a method was added `to_json_dict` which gives the json repr.
This method is internal method which use in 1. Connection Serialisation, and it is a replacement *to_dict* method which was used before https://github.com/apache/airflow/pull/35723 2. Use in public method *as_json* Personally I do not have any personal preference for the naming but if it help to reduce confusion than: "why not?". *as_json* doesn't include in any released version of Airflow, so we could easily change it without any breaking changes: - get_uri -> to_uri_repr, and deprecate get_uri - as_json -> to_json_repr, we don't need to deprecate anything if it added before 2.8.0 On Mon, 27 Nov 2023 at 18:47, Daniel Standish <daniel.stand...@astronomer.io.invalid> wrote: > Just some history ... and a renaming suggestion... > > When get_uri was added, generating the connection URI was a real pain, and > Airflow provided no way to generate one, and JSON repr was not yet > implemented. By adding get_uri, a user could define the connection object > we needed in python, and airflow would give us the URI we needed to set in > our env vars. This was the only purpose. > > I don't think at the time I saw the potential for confusion with the > sqlalchemy URI. In hindsight, get_uri may not be the best name. Recently, > a method was added `to_json_dict` which gives the json repr. Maybe we > should rename get_uri to be consistent and more indicative of the > intention. > > We could rename it `to_uri_str`. Or, maybe better we could make the names > consistent and do `to_json_repr` and `to_uri_repr`. Or something like > that? @andrey WDYT? Adding the _repr suffix may help indicate that this > is the URI (or json) representation of the airflow connection object. > > > > > On Sun, Nov 26, 2023 at 10:52 PM Tzu-ping Chung <t...@astronomer.io.invalid> > wrote: > > > Some context: I’ve been looking into putting the Connection URI to more > > use, most importantly to consolidate it with OpenLineage’s URI format. > > > > Skipping the details, one of the conclusions I had is get_uri on > > Connection is not actually that useful, due to how different libraries > > (SQLAlchemy, Airflow Object Store, OpenLineage, etc.) have drastically > > different URI formats, and it is not practical to implement get_uri in > > Connection to be compatible with all of them. The correct layer to > generate > > a connection URI is in hooks instead, and the format does not need to be > > visible to the user, but internal between the hook and whatever library > it > > talks to, since the URI semantics are too specific to a library to be > > generally useful as a serde format. > > > > Therefore I think deprecating the URI Connection representation would not > > be harmful, except maybe the URI can be more useful in simplistic cases. > > > > > > > On 27 Nov 2023, at 12:21, Amogh Desai <amoghdesai....@gmail.com> > wrote: > > > > > > A little late to the party here but convinced by what Daniel had to > say. > > > > > > I am ok with the idea of adding this into the deprecation bucket in the > > > coming time. > > > > > > Implementing the get_uri in the hook seems quite reasonable to me > > > on the grounds that the Connection class' get_uri is meant to be > general, > > > supporting the use case of serializing the Airflow connection on basis > of > > > host, port, schema, password, type etc. > > > > > > Thanks & Regards, > > > Amogh Desai > > > > > > On Mon, Nov 20, 2023 at 9:56 PM Daniel Standish > > > <daniel.stand...@astronomer.io.invalid> wrote: > > > > > >>> > > >>> I still think different people have different expectations of the > > Airflow > > >>> URI format. My expectation is that this is only a SerDe mechanism > > around > > >>> Airflow Connection. If we have a look at the codebase of Airflow we > > found > > >>> that this is not always true and we try to use it as SA URI. > > >> > > >> > > >> Yes, this is in my view exactly the problem. I believe we run into > > trouble > > >> only when we falsely think that we should be able to define an Airflow > > >> connection using, say, a jdbc connection string. It's not true in > > general. > > >> > > >> Observe that DbApiHook has a get_uri > > >> < > > >> > > > https://github.com/apache/airflow/blob/064fc2b7751a44e37ccce97609cff7c496098e56/airflow/providers/common/sql/hooks/sql.py#L188-L196 > > >>> > > >> method. > > >> > > >> This method is used to produce a connection string valid for the > service > > >> you're connecting to, and importantly it may be different from the > > >> Connection.get_uri. > > >> > > >> The primary responsibility of Airflow connection URI is to serialize > the > > >> airflow connection object which has type, login, pass, host, schema > > port, > > >> extra and that's it. For simple cases it can serve both purposes but > > it's > > >> not true in general and folks need to understand that. > > >> > > >> I think part of the problem may simply be that `uri` is an argument of > > the > > >> Connection object. It means not the connection string but the airflow > > conn > > >> URI serialization. Would probably be a good candidate for deprecation > > >> on the basis that it is confusing. > > >> > > >> Fundamentally, as a hook designer, if you need a conn string that is a > > URI, > > >> you must implement get_uri on the hook in the right way to be able to > > >> use the attributes available to you in Connection. Connection.get_uri > > will > > >> handle creating the Airflow Conn URI serialization and that's a > separate > > >> thing. > > >> > > >> The connection object has no clue how to generate a valid connection > > string > > >> for your service, but the hook can have this understanding. > > >> > > >> There are many examples in the codebase where users implement get_uri > on > > >> the hook and it works just fine. A hook author could write it so you > > can > > >> optionally dump the full connection string in the extra object: > > {"extra": > > >> {"uri": "blah"}}. > > >> > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org > > For additional commands, e-mail: dev-h...@airflow.apache.org > > > > >
