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

refactor(idempotent): Change UX to use a config class for non-persistence related features #306

Merged
merged 14 commits into from Mar 5, 2021

Conversation

michaelbrewer
Copy link
Contributor

@michaelbrewer michaelbrewer commented Mar 4, 2021

Description of changes:

UX for idempotent decorator changes from:

import json
from aws_lambda_powertools.utilities.idempotency import DynamoDBPersistenceLayer, idempotent

# Treat everything under the "body" key in
# the event json object as our payload
persistence_layer = DynamoDBPersistenceLayer(
    event_key_jmespath="body",
    table_name="IdempotencyTable"
)

@idempotent(persistence_store=persistence_layer)
def handler(event, context):
    body = json.loads(event['body'])
    payment = create_subscription_payment(
        user=body['user'],
        product=body['product_id']
    )
    ...
    return {"message": "success", "statusCode": 200, "payment_id": payment.id}

to

import json
from aws_lambda_powertools.utilities.idempotency import DynamoDBPersistenceLayer, IdempotencyConfig, idempotent

# Treat everything under the "body" key in
# the event json object as our payload
config = IdempotencyConfig(event_key_jmespath="body")
persistence_layer = DynamoDBPersistenceLayer(table_name="IdempotencyTable")

@idempotent(config=config, persistence_store=persistence_layer)
def handler(event, context):
    body = json.loads(event['body'])
    payment = create_subscription_payment(
        user=body['user'],
        product=body['product_id']
    )
    ...
    return {"message": "success", "statusCode": 200, "payment_id": payment.id}

All other changes include:

  • Skip the performance tests for the coverage-html
  • pragma: no cover for old uncovered code

Checklist

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

Copy link
Contributor Author

@michaelbrewer michaelbrewer left a comment

Choose a reason for hiding this comment

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

WIP comments

@michaelbrewer michaelbrewer changed the title refactor(idempotent): Create a config class refactor(idempotent): Change UX to use a config class for non-persistence related features Mar 4, 2021
@heitorlessa heitorlessa added area/utilities internal Maintenance changes and removed internal Maintenance changes labels Mar 4, 2021
@michaelbrewer michaelbrewer marked this pull request as ready for review March 4, 2021 17:55
@codecov-io
Copy link

codecov-io commented Mar 4, 2021

Codecov Report

Merging #306 (7d4304b) into develop (153567e) will increase coverage by 0.24%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #306      +/-   ##
===========================================
+ Coverage    99.54%   99.79%   +0.24%     
===========================================
  Files           90       91       +1     
  Lines         3291     3340      +49     
  Branches       160      161       +1     
===========================================
+ Hits          3276     3333      +57     
+ Misses          10        5       -5     
+ Partials         5        2       -3     
Impacted Files Coverage Δ
...ambda_powertools/utilities/idempotency/__init__.py 100.00% <100.00%> (ø)
..._lambda_powertools/utilities/idempotency/config.py 100.00% <100.00%> (ø)
...da_powertools/utilities/idempotency/idempotency.py 98.46% <100.00%> (+0.04%) ⬆️
...wertools/utilities/idempotency/persistence/base.py 99.34% <100.00%> (+5.91%) ⬆️
...ools/utilities/idempotency/persistence/dynamodb.py 100.00% <100.00%> (ø)
aws_lambda_powertools/logging/logger.py 100.00% <0.00%> (ø)
aws_lambda_powertools/tracing/tracer.py 100.00% <0.00%> (ø)
aws_lambda_powertools/metrics/metrics.py 100.00% <0.00%> (ø)
aws_lambda_powertools/logging/formatter.py 100.00% <0.00%> (ø)
aws_lambda_powertools/utilities/batch/base.py 100.00% <0.00%> (ø)
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 153567e...7d4304b. Read the comment docs.

@heitorlessa heitorlessa self-requested a review March 5, 2021 07:42
@heitorlessa heitorlessa self-assigned this Mar 5, 2021
Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

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

One small change for consistency across utilities (or outside the constructor)

@@ -21,7 +21,7 @@ test:
poetry run pytest --cache-clear tests/performance

coverage-html:
poetry run pytest --cov-report html
poetry run pytest -m "not perf" --cov-report=html
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

Comment on lines +4 to +5
class IdempotencyConfig:
def __init__(
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks great - Hopefully when 3.6 is EOL we'll be able to move to dataclasses to make this easier too, including having a generic Config that auto-discovers env vars based on config option name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@heitorlessa should we add a mini project for ideas?

>>> def handler(event, context):
>>> return {"StatusCode": 200}
"""

idempotency_handler = IdempotencyHandler(handler, event, context, persistence_store)
idempotency_handler = IdempotencyHandler(handler, event, context, config or IdempotencyConfig(), persistence_store)
Copy link
Contributor

Choose a reason for hiding this comment

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

to make refactoring easier later, could you please (I can do too): Use or logic outside the instantiation e.g. config = config or IdempotencyConfig()

nitpick: kwargs over args for refactoring too

self._cache: Optional[LRUDict] = None
self.hash_function = None

def configure(self, config: IdempotencyConfig) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking of having this in the constructor and change the way we call it within IdempotencyHandler, but honestly that can be done later - there are a few areas we'll need to refactor, and this will be an internal change either way, and in the worst case we're calling it Beta too, it'll be a minor change IF that.

@heitorlessa
Copy link
Contributor

I don't seem to have permissions to send a quick commit to fix what I asked, so I'm merging as-is, and sending a direct push to develop afterwards

@heitorlessa heitorlessa merged commit 9763bbe into aws-powertools:develop Mar 5, 2021
@michaelbrewer michaelbrewer deleted the refactoring-idempotent-ux branch March 5, 2021 22:08
@heitorlessa
Copy link
Contributor

heitorlessa commented Mar 6, 2021 via email

heitorlessa pushed a commit that referenced this pull request Aug 29, 2022
…60 (#306)

Bumps [mypy-boto3-dynamodb](https://github.com/youtype/mypy_boto3_builder) from 1.24.55.post1 to 1.24.60.
- [Release notes](https://github.com/youtype/mypy_boto3_builder/releases)
- [Commits](https://github.com/youtype/mypy_boto3_builder/commits)

---
updated-dependencies:
- dependency-name: mypy-boto3-dynamodb
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
heitorlessa added a commit that referenced this pull request Aug 29, 2022
…owertools-python into develop

* 'develop' of https://github.com/heitorlessa/aws-lambda-powertools-python:
  chore(deps-dev): bump mypy-boto3-dynamodb from 1.24.55.post1 to 1.24.60 (#306)
rubenfonseca pushed a commit that referenced this pull request Sep 13, 2022
…60 (#306)

Bumps [mypy-boto3-dynamodb](https://github.com/youtype/mypy_boto3_builder) from 1.24.55.post1 to 1.24.60.
- [Release notes](https://github.com/youtype/mypy_boto3_builder/releases)
- [Commits](https://github.com/youtype/mypy_boto3_builder/commits)

---
updated-dependencies:
- dependency-name: mypy-boto3-dynamodb
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
dependabot bot referenced this pull request in ran-isenberg/aws-lambda-powertools-python Sep 15, 2022
…60 (#306)

Bumps [mypy-boto3-dynamodb](https://github.com/youtype/mypy_boto3_builder) from 1.24.55.post1 to 1.24.60.
- [Release notes](https://github.com/youtype/mypy_boto3_builder/releases)
- [Commits](https://github.com/youtype/mypy_boto3_builder/commits)

---
updated-dependencies:
- dependency-name: mypy-boto3-dynamodb
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
github-actions bot referenced this pull request in ran-isenberg/aws-lambda-powertools-python Sep 15, 2022
…60 (#306) (#289)

Bumps [mypy-boto3-dynamodb](https://github.com/youtype/mypy_boto3_builder) from 1.24.55.post1 to 1.24.60.
- [Release notes](https://github.com/youtype/mypy_boto3_builder/releases)
- [Commits](https://github.com/youtype/mypy_boto3_builder/commits)

---
updated-dependencies:
- dependency-name: mypy-boto3-dynamodb
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
dependabot bot referenced this pull request in ran-isenberg/aws-lambda-powertools-python Sep 15, 2022
…60 (#306)

Bumps [mypy-boto3-dynamodb](https://github.com/youtype/mypy_boto3_builder) from 1.24.55.post1 to 1.24.60.
- [Release notes](https://github.com/youtype/mypy_boto3_builder/releases)
- [Commits](https://github.com/youtype/mypy_boto3_builder/commits)

---
updated-dependencies:
- dependency-name: mypy-boto3-dynamodb
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
github-actions bot referenced this pull request in ran-isenberg/aws-lambda-powertools-python Sep 15, 2022
…60 (#306) (#92)

Bumps [mypy-boto3-dynamodb](https://github.com/youtype/mypy_boto3_builder) from 1.24.55.post1 to 1.24.60.
- [Release notes](https://github.com/youtype/mypy_boto3_builder/releases)
- [Commits](https://github.com/youtype/mypy_boto3_builder/commits)

---
updated-dependencies:
- dependency-name: mypy-boto3-dynamodb
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

None yet

3 participants