potiuk commented on PR #24320:
URL: https://github.com/apache/airflow/pull/24320#issuecomment-1156311063

   > @potiuk I think my change in `get_uri` method shouldn’t break something, 
conversely this change will fix current issue. Our `get_uri` function should 
return DB connection URL for `create_engine` function in SQLAlchemy in this 
format `dialect[+driver]://user:password@host/dbname[?key=value..]`. Currently 
Airflow has a problem, when user tries to create DB connection without 
`dbname`, but with `extras` user meets error. It happens, because `get_uri` 
function adds `extras` to the `host` without division by `/` e.g. 
`conn-type://login:password@:3306?charset=utf-8` is wrong URL the correct URL 
should look like this `conn-type://login:password@:3306/?charset=utf-8`.
   
   Could you please point me to the docs/specs where 
`conn-type://login:password@:3306?charset=utf-8` is wrong ? Is it a mysql 
sqlalchemy or "generic" sqlachemy problem?  I am afraid that this **might** 
break something if this is mysql specific limitation rather than generic 
sqlalchemy one. 
   
   FWIW  the URI specification 
https://datatracker.ietf.org/doc/html/rfc3986#section-3.3 is quite clear that 
the `path` component of the URI can be empty, and empty path is different than 
"root path" - (indicated by "/" ) - so I wonder if this change will not have 
impact on any other sqlalchemy URIs  (especially with schema-less databases). 
   
   According to the RFC - `conn-type://login:password@:3306?charset=utf-8` is 
perfectly good URI with "empty" path.
   
   Can you trace down where the limitation of "require `/` path if there is a 
query part" comes from?
   


-- 
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]

Reply via email to