Skip to content
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.

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

#50: Added shallow abstraction for boto3 #51

Merged
merged 8 commits into from
Jun 16, 2023

Conversation

tkilias
Copy link
Collaborator

@tkilias tkilias commented Jun 12, 2023

Fixes #50

@tkilias
Copy link
Collaborator Author

tkilias commented Jun 12, 2023

Integration test for the shallow abstraction will be implemented with a separate ticket #52.

For that reason, please review the boto calls very thoroughly

@tkilias tkilias temporarily deployed to AWS June 12, 2023 17:58 — with GitHub Actions Inactive
@tkilias tkilias temporarily deployed to AWS June 13, 2023 08:23 — with GitHub Actions Inactive
@Nicoretti Nicoretti self-requested a review June 13, 2023 10:21
Copy link
Member

@Nicoretti Nicoretti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some feedback.

Note: I am not familiar enough with AWS to reason about the expected AWS behavior and contracts.

@tkilias tkilias temporarily deployed to AWS June 15, 2023 13:37 — with GitHub Actions Inactive
Comment on lines +7 to +36
def test_arn_exists():
expected = Secret(arn=ARN(aws_arn="EXPECTED_ARN"))
boto_secret = {"ARN": expected.arn.aws_arn}
actual = Secret.from_boto(boto_secret)
assert actual.arn == expected.arn


def test_arn_not_exists():
with pytest.raises(KeyError):
boto_secret = {}
secret = Secret.from_boto(boto_secret)


def test_with_extra_keys():
expected = Secret(arn=ARN(aws_arn="EXPECTED_ARN"))
boto_secret = {
"ARN": expected.arn.aws_arn,
"extra1": None,
"extra2": 1
}
actual = Secret.from_boto(boto_secret)
assert actual == expected


def test_empty_arn():
with pytest.raises(ValueError, match="ARN was None"):
boto_secret = {
"ARN": None
}
secret = Secret.from_boto(boto_secret)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In cases you expect further tests here, they could be rewritten to be parameterized tests. One group of parameterized test for the exception based test and another for the value based ones.

@tkilias tkilias merged commit faad9d9 into main Jun 16, 2023
@tkilias tkilias deleted the refactoring/50_aws_abstraction branch June 16, 2023 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add shallow abstraction for boto
2 participants