You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/09/17 21:05:16 UTC

[GitHub] [airflow] jedcunningham opened a new pull request #18337: Pin `moto` to max version of `2.2.6`

jedcunningham opened a new pull request #18337:
URL: https://github.com/apache/airflow/pull/18337


   `moto==2.2.7` broke a test in main.
   
   e.g:
   
   https://github.com/apache/airflow/runs/3635791248
   
   ```
   tests/providers/amazon/aws/hooks/test_kinesis.py::TestAwsFirehoseHook::test_insert_batch_records_kinesis_firehose: botocore.exceptions.ClientError: An error occurred (UnrecognizedClientException) when calling the CreateDeliveryStream operation: The security token included in the request is invalid.
   ```


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



[GitHub] [airflow] potiuk merged pull request #18337: Fix kinesis test

Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #18337:
URL: https://github.com/apache/airflow/pull/18337


   


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



[GitHub] [airflow] jedcunningham commented on pull request #18337: Pin `moto` to max version of `2.2.6`

Posted by GitBox <gi...@apache.org>.
jedcunningham commented on pull request #18337:
URL: https://github.com/apache/airflow/pull/18337#issuecomment-922100642


   Yeah, just found the same thing. Does requiring 2.2.7+ and using `mock_kinesis` instead sound good?


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



[GitHub] [airflow] mik-laj commented on pull request #18337: Pin `moto` to max version of `2.2.6`

Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #18337:
URL: https://github.com/apache/airflow/pull/18337#issuecomment-922104508


   @jedcunningham  SGTM.  We conditionally import all moto stuff, so we don't even need a lower bound.
   https://github.com/apache/airflow/blob/d7a8a8f60f0fde5923c4974b835e4f2cb1b40b91/tests/providers/amazon/aws/hooks/test_kinesis.py#L24-L27


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



[GitHub] [airflow] potiuk commented on pull request #18337: Pin `moto` to max version of `2.2.6`

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18337:
URL: https://github.com/apache/airflow/pull/18337#issuecomment-922113594


   quick PR to fix it @mik-laj ?


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



[GitHub] [airflow] jedcunningham commented on pull request #18337: Pin `moto` to max version of `2.2.6`

Posted by GitBox <gi...@apache.org>.
jedcunningham commented on pull request #18337:
URL: https://github.com/apache/airflow/pull/18337#issuecomment-922104168


   > Yeah, just found the same thing. Does requiring 2.2.7+ and using `mock_firehose` instead sound good?
   
   That alone doesn't make it pass, unfortunately.


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



[GitHub] [airflow] mik-laj commented on pull request #18337: Pin `moto` to max version of `2.2.6`

Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #18337:
URL: https://github.com/apache/airflow/pull/18337#issuecomment-922112023


   I was able to fix this test.
   ```diiff
   diff --git a/tests/providers/amazon/aws/hooks/test_kinesis.py b/tests/providers/amazon/aws/hooks/test_kinesis.py
   index 52984325e..5b9a1675d 100644
   --- a/tests/providers/amazon/aws/hooks/test_kinesis.py
   +++ b/tests/providers/amazon/aws/hooks/test_kinesis.py
   @@ -19,26 +19,36 @@
    import unittest
    import uuid
    
   +import boto3
   +
    from airflow.providers.amazon.aws.hooks.kinesis import AwsFirehoseHook
    
    try:
   -    from moto import mock_kinesis
   +    from moto import mock_firehose
   +except ImportError:
   +    mock_firehose = None
   +try:
   +    from moto import mock_s3
    except ImportError:
   -    mock_kinesis = None
   +    mock_s3 = None
    
    
    class TestAwsFirehoseHook(unittest.TestCase):
   -    @unittest.skipIf(mock_kinesis is None, 'mock_kinesis package not present')
   -    @mock_kinesis
   +    @unittest.skipIf(mock_firehose is None, 'mock_firehose package not present')
   +    @mock_firehose
        def test_get_conn_returns_a_boto3_connection(self):
            hook = AwsFirehoseHook(
                aws_conn_id='aws_default', delivery_stream="test_airflow", region_name="us-east-1"
            )
            assert hook.get_conn() is not None
    
   -    @unittest.skipIf(mock_kinesis is None, 'mock_kinesis package not present')
   -    @mock_kinesis
   +    @unittest.skipIf(mock_firehose is None, 'mock_firehose package not present')
   +    @unittest.skipIf(mock_s3 is None, 'mock_s3 package not present')
   +    @mock_firehose
   +    @mock_s3
        def test_insert_batch_records_kinesis_firehose(self):
   +        s3_client = boto3.client('s3')
   +        s3_client.create_bucket(Bucket='kinesis-test')
            hook = AwsFirehoseHook(
                aws_conn_id='aws_default', delivery_stream="test_airflow", region_name="us-east-1"
            )
   @@ -55,7 +65,7 @@ class TestAwsFirehoseHook(unittest.TestCase):
            )
    
            stream_arn = response['DeliveryStreamARN']
   -        assert stream_arn == "arn:aws:firehose:us-east-1:123456789012:deliverystream/test_airflow"
   +        assert stream_arn == "arn:aws:firehose:us-east-1:123456789012:/delivery_stream/test_airflow"
    
            records = [{"Data": str(uuid.uuid4())} for _ in range(100)]
    
   
   ```


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



[GitHub] [airflow] github-actions[bot] commented on pull request #18337: Pin `moto` to max version of `2.2.6`

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #18337:
URL: https://github.com/apache/airflow/pull/18337#issuecomment-922083835


   The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.


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



[GitHub] [airflow] mik-laj edited a comment on pull request #18337: Pin `moto` to max version of `2.2.6`

Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #18337:
URL: https://github.com/apache/airflow/pull/18337#issuecomment-922112023


   I was able to fix this test.
   ```diff
   diff --git a/tests/providers/amazon/aws/hooks/test_kinesis.py b/tests/providers/amazon/aws/hooks/test_kinesis.py
   index 52984325e..5b9a1675d 100644
   --- a/tests/providers/amazon/aws/hooks/test_kinesis.py
   +++ b/tests/providers/amazon/aws/hooks/test_kinesis.py
   @@ -19,26 +19,36 @@
    import unittest
    import uuid
    
   +import boto3
   +
    from airflow.providers.amazon.aws.hooks.kinesis import AwsFirehoseHook
    
    try:
   -    from moto import mock_kinesis
   +    from moto import mock_firehose
   +except ImportError:
   +    mock_firehose = None
   +try:
   +    from moto import mock_s3
    except ImportError:
   -    mock_kinesis = None
   +    mock_s3 = None
    
    
    class TestAwsFirehoseHook(unittest.TestCase):
   -    @unittest.skipIf(mock_kinesis is None, 'mock_kinesis package not present')
   -    @mock_kinesis
   +    @unittest.skipIf(mock_firehose is None, 'mock_firehose package not present')
   +    @mock_firehose
        def test_get_conn_returns_a_boto3_connection(self):
            hook = AwsFirehoseHook(
                aws_conn_id='aws_default', delivery_stream="test_airflow", region_name="us-east-1"
            )
            assert hook.get_conn() is not None
    
   -    @unittest.skipIf(mock_kinesis is None, 'mock_kinesis package not present')
   -    @mock_kinesis
   +    @unittest.skipIf(mock_firehose is None, 'mock_firehose package not present')
   +    @unittest.skipIf(mock_s3 is None, 'mock_s3 package not present')
   +    @mock_firehose
   +    @mock_s3
        def test_insert_batch_records_kinesis_firehose(self):
   +        s3_client = boto3.client('s3')
   +        s3_client.create_bucket(Bucket='kinesis-test')
            hook = AwsFirehoseHook(
                aws_conn_id='aws_default', delivery_stream="test_airflow", region_name="us-east-1"
            )
   @@ -55,7 +65,7 @@ class TestAwsFirehoseHook(unittest.TestCase):
            )
    
            stream_arn = response['DeliveryStreamARN']
   -        assert stream_arn == "arn:aws:firehose:us-east-1:123456789012:deliverystream/test_airflow"
   +        assert stream_arn == "arn:aws:firehose:us-east-1:123456789012:/delivery_stream/test_airflow"
    
            records = [{"Data": str(uuid.uuid4())} for _ in range(100)]
    
   
   ```


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



[GitHub] [airflow] jedcunningham edited a comment on pull request #18337: Pin `moto` to max version of `2.2.6`

Posted by GitBox <gi...@apache.org>.
jedcunningham edited a comment on pull request #18337:
URL: https://github.com/apache/airflow/pull/18337#issuecomment-922100642


   Yeah, just found the same thing. Does requiring 2.2.7+ and using `mock_firehose` instead sound good?


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



[GitHub] [airflow] jedcunningham commented on pull request #18337: Fix kinesis test

Posted by GitBox <gi...@apache.org>.
jedcunningham commented on pull request #18337:
URL: https://github.com/apache/airflow/pull/18337#issuecomment-922123412


   > We conditionally import all moto stuff, so we don't even need a lower bound.
   
   I bumped the lower bound just to be defensive - when I uninstalled `moto` it blew up elsewhere. Something to look into another day.


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



[GitHub] [airflow] mik-laj commented on pull request #18337: Pin `moto` to max version of `2.2.6`

Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #18337:
URL: https://github.com/apache/airflow/pull/18337#issuecomment-922099576


   It is caused by https://github.com/spulec/moto/pull/4246
   Problably, we should use `mock_firehose` instead `mock_kinesis`.


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