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:
us...@infra.apache.org



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

Reply via email to