Skip to content

Remove deprecated modules from Amazon provider package #25609

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
potiuk merged 6 commits into from
Aug 9, 2022
Merged

Remove deprecated modules from Amazon provider package #25609

potiuk merged 6 commits into from
Aug 9, 2022

Conversation

@vincbeck
Copy link
Contributor

@vincbeck vincbeck commented Aug 8, 2022 •
edited
Loading

Follow up of #25543. @potiuk

@boring-cyborg boring-cyborg bot added area:providers provider:amazon AWS/Amazon - related issues labels Aug 8, 2022
@vincbeck vincbeck mentioned this pull request Aug 8, 2022
Merged
@potiuk
Copy link
Member

potiuk commented Aug 8, 2022

Yep. Waiting for the tests to complete :)

@potiuk
Copy link
Member

potiuk commented Aug 8, 2022

stilll a number of tests failing :( @vincbeck

@Taragolis
Copy link
Contributor

Taragolis commented Aug 8, 2022 •
edited
Loading

@vincbeck

Selected tests not relevant anymore and could be removed

test_get_client_type_returns_a_boto3_client_of_the_requested_type

airflow/tests/providers/amazon/aws/hooks/test_base_aws.py

Lines 223 to 232 in 72a6ac5

@unittest.skipIf(mock_emr is None, 'mock_emr package not present')
@mock_emr
def test_get_client_type_returns_a_boto3_client_of_the_requested_type(self):
client = boto3.client('emr', region_name='us-east-1')
if client.list_clusters()['Clusters']:
raise ValueError('AWS not properly mocked')
hook = AwsBaseHook(aws_conn_id='aws_default', client_type='emr')
client_from_hook = hook.get_client_type('emr')
assert client_from_hook.list_clusters()['Clusters'] == []

test_get_client_type_overwrite

airflow/tests/providers/amazon/aws/hooks/test_base_aws.py

Lines 245 to 253 in 72a6ac5

@unittest.skipIf(mock_emr is None, 'mock_emr package not present')
@mock_emr
def test_get_client_type_overwrite(self):
client = boto3.client('emr', region_name='us-east-1')
if client.list_clusters()['Clusters']:
raise ValueError('AWS not properly mocked')
hook = AwsBaseHook(aws_conn_id='aws_default', client_type='dynamodb')
client_from_hook = hook.get_client_type(client_type='emr')
assert client_from_hook.list_clusters()['Clusters'] == []

test_get_client_type_deprecation_warning

airflow/tests/providers/amazon/aws/hooks/test_base_aws.py

Lines 255 to 262 in 72a6ac5

@unittest.skipIf(mock_emr is None, 'mock_emr package not present')
@mock_emr
def test_get_client_type_deprecation_warning(self):
hook = AwsBaseHook(aws_conn_id='aws_default', client_type='emr')
warning_message = """client_type is deprecated. Set client_type from class attribute."""
with pytest.warns(DeprecationWarning) as warnings:
hook.get_client_type(client_type='emr')
assert warning_message in [str(w.message) for w in warnings]

test_get_resource_type_returns_a_boto3_resource_of_the_requested_type

airflow/tests/providers/amazon/aws/hooks/test_base_aws.py

Lines 264 to 282 in 72a6ac5

@unittest.skipIf(mock_dynamodb2 is None, 'mock_dynamo2 package not present')
@mock_dynamodb2
def test_get_resource_type_returns_a_boto3_resource_of_the_requested_type(self):
hook = AwsBaseHook(aws_conn_id='aws_default', resource_type='dynamodb')
resource_from_hook = hook.get_resource_type('dynamodb')
# this table needs to be created in production
table = resource_from_hook.create_table(
TableName='test_airflow',
KeySchema=[
{'AttributeName': 'id', 'KeyType': 'HASH'},
],
AttributeDefinitions=[{'AttributeName': 'id', 'AttributeType': 'S'}],
ProvisionedThroughput={'ReadCapacityUnits': 10, 'WriteCapacityUnits': 10},
)
table.meta.client.get_waiter('table_exists').wait(TableName='test_airflow')
assert table.item_count == 0

test_get_resource_type_overwrite

airflow/tests/providers/amazon/aws/hooks/test_base_aws.py

Lines 304 to 322 in 72a6ac5

@unittest.skipIf(mock_dynamodb2 is None, 'mock_dynamo2 package not present')
@mock_dynamodb2
def test_get_resource_type_overwrite(self):
hook = AwsBaseHook(aws_conn_id='aws_default', resource_type='s3')
resource_from_hook = hook.get_resource_type('dynamodb')
# this table needs to be created in production
table = resource_from_hook.create_table(
TableName='test_airflow',
KeySchema=[
{'AttributeName': 'id', 'KeyType': 'HASH'},
],
AttributeDefinitions=[{'AttributeName': 'id', 'AttributeType': 'S'}],
ProvisionedThroughput={'ReadCapacityUnits': 10, 'WriteCapacityUnits': 10},
)
table.meta.client.get_waiter('table_exists').wait(TableName='test_airflow')
assert table.item_count == 0

test_get_resource_deprecation_warning

airflow/tests/providers/amazon/aws/hooks/test_base_aws.py

Lines 324 to 331 in 72a6ac5

@unittest.skipIf(mock_dynamodb2 is None, 'mock_dynamo2 package not present')
@mock_dynamodb2
def test_get_resource_deprecation_warning(self):
hook = AwsBaseHook(aws_conn_id='aws_default', resource_type='dynamodb')
warning_message = """resource_type is deprecated. Set resource_type from class attribute."""
with pytest.warns(DeprecationWarning) as warnings:
hook.get_resource_type('dynamodb')
assert warning_message in [str(w.message) for w in warnings]

test_refreshable_credentials uses removed attribute

Quick hotfix:

  1. In hook specify client_type='sts'

    airflow/tests/providers/amazon/aws/hooks/test_base_aws.py

    Lines 623 to 625 in 72a6ac5

    mock_get_connection.return_value = mock_connection
    hook = AwsBaseHook(aws_conn_id='aws_default', client_type='airflow_test')

  2. hook.get_client_type('sts') -> hook.get_client_type()

    airflow/tests/providers/amazon/aws/hooks/test_base_aws.py

    Lines 662 to 664 in 72a6ac5

    mock_refresh.side_effect = mock_refresh_credentials
    client = hook.get_client_type('sts')
    client.get_caller_identity()

@vincbeck
Copy link
Contributor Author

vincbeck commented Aug 8, 2022

Awesome! Thank you so much @Taragolis

@eladkal eladkal self-requested a review August 9, 2022 06:17
@eladkal
Copy link
Contributor

eladkal commented Aug 9, 2022

CI is not happy :(
some failures related to TestSsmSecrets

eladkal
eladkal reviewed Aug 9, 2022
View reviewed changes
Taragolis
Taragolis reviewed Aug 9, 2022
View reviewed changes
@potiuk
Copy link
Member

potiuk commented Aug 9, 2022

I prepared the #25618 - but I can still include that one if we fix the issues

@potiuk
Copy link
Member

potiuk commented Aug 9, 2022

🤞 @vincbeck - I think this is the last one I want to include in this wave of providers :) (I will make one more pass to see)

@vincbeck
Copy link
Contributor Author

vincbeck commented Aug 9, 2022

Some tests were canceled. Any idea @potiuk ?

@potiuk
Copy link
Member

potiuk commented Aug 9, 2022

Just one - likely an intermittent error. Re-run it @vincbeck

@vincbeck
Copy link
Contributor Author

vincbeck commented Aug 9, 2022

YAY!

@potiuk potiuk merged commit 85137f3 into apache:main Aug 9, 2022
@potiuk
Copy link
Member

potiuk commented Aug 9, 2022

🎉 🎉 🎉 🎉 🎉

@vincbeck vincbeck deleted the vincbeck/deprecation2 branch August 9, 2022 19:45
@eladkal eladkal mentioned this pull request Aug 10, 2022
Merged
This was referenced Aug 10, 2022
Closed
Closed
@ephraimbuddy ephraimbuddy added the type:misc/internal Changelog: Misc changes that should appear in change log label Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Taragolis Taragolis Taragolis left review comments

@eladkal eladkal eladkal left review comments

@potiuk potiuk potiuk approved these changes

Assignees

No one assigned

Labels

area:providers provider:amazon AWS/Amazon - related issues type:misc/internal Changelog: Misc changes that should appear in change log

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@vincbeck @potiuk @Taragolis @eladkal @ephraimbuddy