[GitHub] [airflow] potiuk commented on pull request #33715: allow impersonation_chain to be set on Google Cloud connection

["potiuk (via GitHub)" <gi...@apache.org>]

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

   I don't see a big problem with it (though it misses tests and they should be added).
   
   I wonder if it won't have side effects. Currently Impersonation chain is really "per-operator" feature not "per-connection" one. Whichever operator supports impersonation chain, they pass it down to a relevant hook and then it is passed down to the hook that uses it. Indeed we started where impersonation chain was supported by just a handful of operators and services but apparently now - more of them have it and we **could** make it a connection feature indeed.
   
   There are however some interesting back-compatibility problems. Users for example might expect that when they set it in the connection, it will be used for all services, but they might already have impersonation chain set in the operator (this is currently the only way they can do it.  
   
   So this "connection setting" might be misleading for past users and transition will be a bit painful because what the users will really have to do, in case they want to change it to a connection-driven impersonation chain - they will have to modify all the DAGs they currently have, otherwise this will be confusing because some of their dags might use the old way and some the new way.
   
   So maybe - if we introduce we could come up with some ideas on how to make such migration easier. Quite for sure in order to add it, you will have to add a migration documentation - i.e. explain the users what they should do in case they need to convert to this new method, maybe even also some warnings and inforamation in logs that the "connection" impersonation is not being used and is overwritten. Not sure if that is that important, though.
   
   I wonder what @VladaZakharova and @bkossakowska and maybe and other from the team of google operators think about it. Maybe also @hussein-awala  who was implementing some Google connection improvements.
   
   


-- 
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: commits-unsubscribe@airflow.apache.org

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