[GitHub] [airflow] mrshu commented on issue #5879: [AIRFLOW-5280] conn: Remove aws_default's default region name
mrshu commented on issue #5879: [AIRFLOW-5280] conn: Remove aws_default's default region name URL: https://github.com/apache/airflow/pull/5879#issuecomment-535940021 @ashb It will not work in the sense that you will additionally need to specify either the `region_name` in the extra parameters (which the [docs](https://airflow.apache.org/howto/connection/aws.html#configuring-the-connection) mention) or set the `AWS_DEFAULT_REGION` environment variable. I would argue that the `aws_default` connection with "bare" install does not work for anyone who operates outside of the `us-east-1`. I realize it is special for historical reasons but with [22 regions](https://aws.amazon.com/about-aws/global-infrastructure/), I would wager that many will be hit by the unexpected behavior I described in the PR description: that you won't realize Airflow is silently using `us-east-1` as the default region without digging through the code. I firmly believe that explicit is better than implicit, especially in cases of default configuration. Hence I see it more as fixing a small bug than changing some already-established behavior. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] mrshu commented on issue #5879: [AIRFLOW-5280] conn: Remove aws_default's default region name
mrshu commented on issue #5879: [AIRFLOW-5280] conn: Remove aws_default's default region name URL: https://github.com/apache/airflow/pull/5879#issuecomment-535653906 @ashb `UPDATING.md` ran into a conflicting situation again in the meantime but I have resolved it with the last commit on the list. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] mrshu commented on issue #5879: [AIRFLOW-5280] conn: Remove aws_default's default region name
mrshu commented on issue #5879: [AIRFLOW-5280] conn: Remove aws_default's default region name URL: https://github.com/apache/airflow/pull/5879#issuecomment-534745161 @ashb Sorry for the delay here, both the conflict and the whitespace issues should be resolved here. The PR should be otherwise (hopefully) ready. Thank you for your patience again! 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] mrshu commented on issue #5879: [AIRFLOW-5280] conn: Remove aws_default's default region name
mrshu commented on issue #5879: [AIRFLOW-5280] conn: Remove aws_default's default region name URL: https://github.com/apache/airflow/pull/5879#issuecomment-533715114 @ashb I've just noticed there was a conflict in the `UPDATING.md` file -- I've just resolved it. Please do let me know if I can do anything else here. Thanks! 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] mrshu commented on issue #5879: [AIRFLOW-5280] conn: Remove aws_default's default region name
mrshu commented on issue #5879: [AIRFLOW-5280] conn: Remove aws_default's default region name URL: https://github.com/apache/airflow/pull/5879#issuecomment-532199570 Thanks for all the help here @ashb! 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] mrshu commented on issue #5879: [AIRFLOW-5280] conn: Remove aws_default's default region name
mrshu commented on issue #5879: [AIRFLOW-5280] conn: Remove aws_default's default region name URL: https://github.com/apache/airflow/pull/5879#issuecomment-528215885 Thanks @ashb, I've added a few lines to `UPDATING.md` -- once the tests pass (which they should momentarily), this should be good to go. Note that the `AWS_DEFAULT_REGION` env variables that are exported before execution of tests now inherit from the shell they are being executed in (e.g. when `AWS_DEFAULT_REGION` is already exported, the value will be reused, otherwise `us-east-1` will be used). The tests seem to be failing here sporadically, but mostly because the `docker run -i --rm -v /home/travis/build/apache/airflow/scripts/ci/kubernetes/minikube:/build ubuntu:14.04` step does not produce any output in 10 minutes. Would you mind advising me how to deal with that? Thanks! 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] mrshu commented on issue #5879: [AIRFLOW-5280] conn: Remove aws_default's default region name
mrshu commented on issue #5879: [AIRFLOW-5280] conn: Remove aws_default's default region name URL: https://github.com/apache/airflow/pull/5879#issuecomment-524446941 @ashb I got the tests to pass once `AWS_DEFAULT_REGION` has been exported in proper places. I also added a quick note on this change to the AWS connection docs. Let me try to reiterate once again why do I think this change should be applied: When you install Airflow right now and use either the `AIRFLOW_CONN_` environment variable or the `~/.aws/credentials` to set the AWS connection parameters, the region setting gets ignored and `us-east-1` gets used. The only way of finding that out is diving deep into `botocore` and trying to debug the values it gets passed by Airflow, which is what prompted me to create this Pull Request. Just documenting the fact would be certainly helpful, but I believe this change would be much better: in the case I outlined above you would receive a `NoRegionSet` error which is much more informational than the current state of silently falling back to `us-east-1`. That said, if you do not agree with this change for any reason, I am happy to close this PR and create a new one that would only add the documentation bit. Thanks again! 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] mrshu commented on issue #5879: [AIRFLOW-5280] conn: Remove aws_default's default region name
mrshu commented on issue #5879: [AIRFLOW-5280] conn: Remove aws_default's default region name URL: https://github.com/apache/airflow/pull/5879#issuecomment-523861473 @ashb I am also happy to just update the docs if you think that would be better -- it's just that I haven't found this documented anywhere and do not see a significant advantage in keeping it the way it currently is. My scope is just quite limited, so I am happy to be proved wrong :) Thanks! 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: us...@infra.apache.org With regards, Apache Git Services
