[GitHub] [airflow] dstandish edited a comment on issue #6440: [AIRFLOW-5768] GCP cloud sql don't store ephemeral connection in db

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

dstandish edited a comment on issue #6440: [AIRFLOW-5768] GCP cloud sql don't store ephemeral connection in db
URL: https://github.com/apache/airflow/pull/6440#issuecomment-546762456
 
 
   > It _might_ be worth moving some of that logic down in to the base DbApiHook? Not sure.
   
   Yeah I thought about this too and was also unsure.  Just "connection" is a term that has great potential for conflict / confusion / ambiguity, to add it to all hooks could be trouble.  Maybe if we renamed `Connection` to `AirflowConnection`...  Or perhaps better, `Creds` or `Credential`.  `Connection` is not really a connection.  This causes a lot of confusing code, come to think of it (all the `get_conn` in the codebase).  
   
   An alternative approach would be to alter the hook to accept every param the mysql connections (actual connection) would need, so that the GCP Cloud SQL hook does not actually need to create a connection object -- it can just pass the appropriate params to postgres / mysql hook.  
   
   On this latter idea, others have discouraged adding params like `password` as init params for hook on the basis it would allow users to do bad things like put creds in the dag file under source control.  I personally don't feel like we should be nanny state in that way.  A novice user might still put `pyodbc.connect` in their dags, for example.  So I don't see this as a reason not to add params.

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


With regards,
Apache Git Services