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

Feature/python hexagonal arch #107

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

Conversation

rohanmeh
Copy link
Contributor

Issue #, if available:

Description of changes:
Python Hexagonal Architecture Example

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@rohanmeh rohanmeh requested a review from wtromano-aws April 10, 2023 19:53
Copy link
Contributor

@dancfox dancfox left a comment

Choose a reason for hiding this comment

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

Will do another pass once these fixes are made.

python-test-samples/hexagonal-architectures/README.md Outdated Show resolved Hide resolved
python-test-samples/hexagonal-architectures/README.md Outdated Show resolved Hide resolved
4. Primary actors: Users of the system such as a webhook, a UI request, or a test script.
5. Secondary actors: used by the application, these services are either a Repository (for example, a database) or a Recipient (such as a message queue).

Application Description
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be a markdown header?

python-test-samples/hexagonal-architectures/README.md Outdated Show resolved Hide resolved

## Run the Integration Test

(Coming Soon)
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove?

Copy link
Contributor

@dancfox dancfox left a comment

Choose a reason for hiding this comment

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

I made some very minor suggestions in the README. The tests passed for me.

Should we illustrate some specifics of the feature side of the code (/src directory) of how the ports and adapters pattern has improved our ability to test?

I'm not an expert in ports and adapters theory. Let's see if we can get Luca to take a look at this PR and comment. I will reach out to him.

## Run the Unit Test
[mock_test.py](tests/unit/mock_test.py)

In the [unit test](tests/unit/mock_test.py), all references and calls to the DynamoDB service [are mocked on line 22](tests/unit/mock_test.py#L22).
Copy link
Contributor

Choose a reason for hiding this comment

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

Line 21 of the unit test initializes a decorator that the mock framework moto uses to intercept HTTP requests sent to DynamoDB by boto3.


The unit test establishes the STOCKS_DB_TABLE and CURRENCIES_DB_TABLE environment variables that the Lambda function uses to reference the DynamoDB tables. STOCKS_DB_TABLE and CURRENCIES_DB_TABLE are defined in the [setUp method of test class in mock_test.py](tests/unit/mock_test.py#L29-81).

In a unit test, you must create a mocked version of the DynamoDB table. The example approach in the [setUp method of test class in mock_test.py](tests/unit/mock_test.py#L43-50) reads in the DynamoDB table schema directly the [SAM Template](template.yaml) so that the definition is maintained in one place. This simple technique works if there are no intrinsics (like !If or !Ref) in the resource properties for KeySchema, AttributeDefinitions, & BillingMode. Once the mocked table is created, test data is populated.
Copy link
Contributor

Choose a reason for hiding this comment

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

The unit test mocks all calls to DynamoDB. The example approach in the setUp method of test class in mock_test.py reads in the DynamoDB table schema directly from the SAM Template so that the definition is maintained in one place.

@wtromano-aws
Copy link
Contributor

I agree - and a diagram of the ports and adapters model and map to the sample code would be helpful. I'll hold for Luca's suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants