[GitHub] [airflow] Taragolis commented on pull request #25494: `AwsBaseHook` not using Connection's host

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

Taragolis commented on PR #25494:
URL: https://github.com/apache/airflow/pull/25494#issuecomment-1203611035

   My personal thought both of `Connection.host` and `Connection.extra['host']` not a good candidate for `endpoint_url`.
   
   I would rather deprecate  `Connection.extra['host']` by `Connection.extra['endpoint_url']`:
   1. In this case it would be easier implement #25326 
   2. keep same name as it use in [boto3.client](https://boto3.amazonaws.com/v1/documentation/api/latest/reference/core/session.html#boto3.session.Session.client) and [boto3.resource](https://boto3.amazonaws.com/v1/documentation/api/latest/reference/core/session.html#boto3.session.Session.resource)
   
   Please also note that connection which read from URI (Environment Variables, SSM backend and might be other sources) would [always contain non-None](https://github.com/apache/airflow/blob/298be502c35006b7c3f011b676dbb4db0633bc74/airflow/models/connection.py#L50-L60) `host`. In other hand It might be an issue if we pass `endpoint_url=''` rather than `endpoint_url=None` to boto3
   
   ```python
   from airflow.models.connection import Connection
   
   conn = Connection(uri="aws://")
   assert conn.host is None, f"Expected None but got {conn.host!r}"
   ```
   
   Also feel free to join discussion in this PR #25416 which basically hide unused connections fields from UI, since it not used in `AwsConnectionWrapper` and we do not provide `host` attribute as compatibility for Connection object.


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