-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Description
Apache Airflow version:
Version: v2.1.2
Git Version: .release:2.1.2+d25854dd413aa68ea70fb1ade7fe01425f456192
OS:
PRETTY_NAME="Debian GNU/Linux 10 (buster)"
NAME="Debian GNU/Linux"
VERSION_ID="10"
VERSION="10 (buster)"
VERSION_CODENAME=buster
ID=debian
HOME_URL="https://www.debian.org/"
SUPPORT_URL="https://www.debian.org/support"
BUG_REPORT_URL="https://bugs.debian.org/"
Apache Airflow Provider versions:
Deployment:
Docker compose using reference docker compose from here: https://airflow.apache.org/docs/apache-airflow/stable/start/docker.html
What happened:
Connection specifications have a host field. This is declared as a member variable of the connection class here:
airflow/airflow/models/connection.py
Line 101 in 96f7e3f
| host = Column(String(500)) |
It's correctly used in the BaseHook class, eg here:
Line 69 in 96f7e3f
| if conn.host: |
However in AwsBaseHook, it's expected to be in extra, here:
airflow/airflow/providers/amazon/aws/hooks/base_aws.py
Lines 404 to 406 in 96f7e3f
| connection_object = self.get_connection(self.aws_conn_id) | |
| extra_config = connection_object.extra_dejson | |
| endpoint_url = extra_config.get("host") |
This should be changed to
endpoint_url = connection_object.host
What you expected to happen:
AwsBaseHook should use the connection.host value, not connection.extra.host
How to reproduce it:
See above code
Anything else we need to know:
Are you willing to submit a PR?
Probably. It'd also be good for someone inexperienced. This would represent a backwards-incompatible change, any problem with that?