Repository: incubator-airflow Updated Branches: refs/heads/master e9a09c271 -> 8e5805399
[AIRFLOW-2771] Add except type to broad S3Hook try catch clauses S3Hook will silently fail if given a conn_id that does not exist. The calls to check_for_key done by an S3KeySensor will never fail if the credentials object is not configured correctly. This adds the expected ClientError exception type when performing a HEAD operation on an object that doesn't exist to the try catch statements so that other exceptions are properly raised. Closes #3616 from mascah/AIRFLOW-2771-S3hook- except-type Project: http://git-wip-us.apache.org/repos/asf/incubator-airflow/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-airflow/commit/8e580539 Tree: http://git-wip-us.apache.org/repos/asf/incubator-airflow/tree/8e580539 Diff: http://git-wip-us.apache.org/repos/asf/incubator-airflow/diff/8e580539 Branch: refs/heads/master Commit: 8e58053992aa8fd93d27283c8d97ff24491a68a8 Parents: e9a09c2 Author: Mike Ascah <[email protected]> Authored: Fri Jul 20 13:46:50 2018 +0200 Committer: Fokko Driesprong <[email protected]> Committed: Fri Jul 20 13:46:50 2018 +0200 ---------------------------------------------------------------------- airflow/hooks/S3_hook.py | 7 +++++-- tests/hooks/test_s3_hook.py | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/8e580539/airflow/hooks/S3_hook.py ---------------------------------------------------------------------- diff --git a/airflow/hooks/S3_hook.py b/airflow/hooks/S3_hook.py index b4f3ac3..5e505ca 100644 --- a/airflow/hooks/S3_hook.py +++ b/airflow/hooks/S3_hook.py @@ -16,6 +16,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from botocore.exceptions import ClientError from airflow.exceptions import AirflowException from airflow.contrib.hooks.aws_hook import AwsHook @@ -54,7 +55,8 @@ class S3Hook(AwsHook): try: self.get_conn().head_bucket(Bucket=bucket_name) return True - except: + except ClientError as e: + self.log.info(e.response["Error"]["Message"]) return False def get_bucket(self, bucket_name): @@ -168,7 +170,8 @@ class S3Hook(AwsHook): try: self.get_conn().head_object(Bucket=bucket_name, Key=key) return True - except: + except ClientError as e: + self.log.info(e.response["Error"]["Message"]) return False def get_key(self, key, bucket_name=None): http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/8e580539/tests/hooks/test_s3_hook.py ---------------------------------------------------------------------- diff --git a/tests/hooks/test_s3_hook.py b/tests/hooks/test_s3_hook.py index baac203..27dfba4 100644 --- a/tests/hooks/test_s3_hook.py +++ b/tests/hooks/test_s3_hook.py @@ -21,6 +21,8 @@ import mock import unittest +from botocore.exceptions import NoCredentialsError + from airflow import configuration try: @@ -28,7 +30,6 @@ try: except ImportError: S3Hook = None - try: import boto3 from moto import mock_s3 @@ -51,6 +52,7 @@ class TestS3Hook(unittest.TestCase): self.assertEqual(parsed, ("test", "this/is/not/a-real-key.txt"), "Incorrect parsing of the s3 url") + @mock_s3 def test_check_for_bucket(self): hook = S3Hook(aws_conn_id=None) @@ -60,6 +62,12 @@ class TestS3Hook(unittest.TestCase): self.assertTrue(hook.check_for_bucket('bucket')) self.assertFalse(hook.check_for_bucket('not-a-bucket')) + def test_check_for_bucket_raises_error_with_invalid_conn_id(self): + hook = S3Hook(aws_conn_id="does_not_exist") + + with self.assertRaises(NoCredentialsError): + hook.check_for_bucket('bucket') + @mock_s3 def test_get_bucket(self): hook = S3Hook(aws_conn_id=None) @@ -146,6 +154,12 @@ class TestS3Hook(unittest.TestCase): self.assertFalse(hook.check_for_key('b', 'bucket')) self.assertFalse(hook.check_for_key('s3://bucket//b')) + def test_check_for_key_raises_error_with_invalid_conn_id(self): + hook = S3Hook(aws_conn_id="does_not_exist") + + with self.assertRaises(NoCredentialsError): + hook.check_for_key('a', 'bucket') + @mock_s3 def test_get_key(self): hook = S3Hook(aws_conn_id=None)
