[GitHub] [incubator-superset] zhaoyongjie edited a comment on pull request #11233: chore(engine): adapt postgres backend connection URI

[GitBox <gi...@apache.org>]

zhaoyongjie edited a comment on pull request #11233:
URL: https://github.com/apache/incubator-superset/pull/11233#issuecomment-708131378


   > > 1. There are already many places where database.spec.engine is used to get dialect name. If we modify the engine datetype, it means that many places need to be adjusted accordingly
   > 
   > I may be wrong, but the db engine specs are mostly used to get values back without needing to check for what the engine itself is. Was there some place in the code that referenced the value of `BaseEngineSpec.engine` directly, other than `db_engine_specs/__init__.py`?
   
   In superset.views.core have some spec.engine to get engine name, like: 
   - [L1951](https://github.com/apache/incubator-superset/blob/2c649ac20f049804b1e52259add4352282d1b28c/superset/views/core.py#L1951) 
   - [L2087](https://github.com/apache/incubator-superset/blob/2c649ac20f049804b1e52259add4352282d1b28c/superset/views/core.py#L2087)
   - [L2091](https://github.com/apache/incubator-superset/blob/2c649ac20f049804b1e52259add4352282d1b28c/superset/views/core.py#L2091)
   
   ![image](https://user-images.githubusercontent.com/2016594/95939658-27cd5b00-0e0f-11eb-8c3b-def9c6cb49fb.png)
   
   
   > 
   > > 1. SQLAlchemy dialect name received is a string datatype. ref: https://github.com/zzzeek/sqlalchemy/blob/34f8622b687f33dadff4f8e81532b3f4050246f4/lib/sqlalchemy/engine/interfaces.py#L38
   > 
   > This shouldn't cause trouble, as we'll be mapping all values in the list to the engines dict in
   > 
   > https://github.com/apache/incubator-superset/blob/8676c3ebd6f49fd8102e68d682939728184b5638/superset/db_engine_specs/__init__.py#L51
   > 
   > > I refer to SQLAlchemy "translate" method support "postgres" and "postgresql". https://github.com/zzzeek/sqlalchemy/blob/6eda3aa8c5f58e415a49ac8dc73eb8608f4be306/lib/sqlalchemy/dialects/__init__.py#L21
   > 
   > Hmm, I wonder if we could leverage this to automatically populate the dialect aliases..
   
   If we modify the engine datatype, these places may need to be adjusted....
   
   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org