dabla commented on PR #69969:
URL: https://github.com/apache/airflow/pull/69969#issuecomment-4999853492

   > Before making the `BaseHook`change, it is worth checking whether this is 
actually needed. Airflow's secrets-backend contract already guarantees that 
`Connection.conn_id` on the object returned by `get_connection()` matches the 
requested lookup key, see: 
https://github.com/apache/airflow/blob/main/shared/secrets_backend/src/airflow_shared/secrets_backend/base.py#L121-L157
   > 
   > Both construct the `Connection` using the requested `conn_id`, not 
anything derived from the secret's contents. Every stock backend such as 
`EnvironmentVariablesBackend`, `MetastoreBackend`, `LocalFilesystemBackend`, 
and the Hashicorp `VaultBackend` only override `get_conn_value`and never 
touches `conn_id`. That is how alias style translation is supposed to work: the 
backend uses the alias to find the right underlying secret, but the Connection 
object it hands back still identifies itself by the alias.
   > 
   > The custom backend that you describe in this PR might be overriding 
`get_connection`/`deserialize_connection` (or constructing Connection some 
other way) and explicitly stamping the canonical, environment specific id onto 
`conn_id` which is a deviation from the contract every other backend honors, 
not a gap in `BaseHook` as I see it
   > 
   > Can you check in the custom backend for above and have it return 
`Connection(conn_id=<alias>, ...)` and keep the canonical/resolved id purely as 
an internal lookup detail?
   
   Good point @amoghrajesh, I will try this change in our custom provider, if 
it fixes the issue, then I'll close this PR.


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