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

feat(event-handler): add appsync batch invoke #1998

Open
wants to merge 82 commits into
base: develop
Choose a base branch
from

Conversation

mploski
Copy link
Contributor

@mploski mploski commented Mar 10, 2023

Issue Number: #1303

Summary

Support Appsync batching mechanism for Lambda Resolvers

Changes

  • Modify Appsync BaseRouter to accept List of AppSyncResolverEventT events alongside singular event
  • Store List of AppSyncResolverEventT events in BaseRouter.current_event
  • Add Functional test covering batch processing
  • Add aws-cdk-aws-appsync-alpha cdk package
  • Add end2end tests that
    • deploys AppSync API with lambda resolver and specific schema
    • Validates different resolvers including batching mechanism

User experience

If batch processing is used for specific method, user can access AppSyncResolver events directly in their annotated method and do appropriate processing. Appsync resolver calls method for every event from the coming list of events:

from typing import List, Optional

from pydantic import BaseModel

from aws_lambda_powertools.event_handler import AppSyncResolver
from aws_lambda_powertools.utilities.data_classes import AppSyncResolverEvent
from aws_lambda_powertools.utilities.typing import LambdaContext

app = AppSyncResolver()


posts = {
    "1": {
        "post_id": "1",
        "title": "First book",
        "author": "Author1",
        "url": "https://amazon.com/",
        "content": "SAMPLE TEXT AUTHOR 1",
        "ups": "100",
        "downs": "10",
    },
    "2": {
        "post_id": "2",
        "title": "Second book",
        "author": "Author2",
        "url": "https://amazon.com",
        "content": "SAMPLE TEXT AUTHOR 2",
        "ups": "100",
        "downs": "10",
    },
    "3": {
        "post_id": "3",
        "title": "Third book",
        "author": "Author3",
        "url": None,
        "content": None,
        "ups": None,
        "downs": None,
    }
}

posts_related = {
    "1": [posts["2"]],
    "2": [posts["3"], posts["1"]],
    "3": [posts["2"], posts["1"]],
}


class Post(BaseModel):
    post_id: str
    author: str
    title: str
    url: str
    content: str
    ups: str
    downs: str

@app.batch_resolver(type_name="Post", field_name="relatedPosts")
def related_posts(event: AppSyncResolverEvent) -> Optional[list]:
    return posts_related[event.source["post_id"]] if event.source else None


def lambda_handler(event, context: LambdaContext) -> dict:
    return app.resolve(event, context)

Under the hood, Router:

  1. Recognize we got a list of events so it assumes request coming from appsync batch operations.
  2. It registers methods that we want to use to handle batch events. We use for it brand new resolver called batch_resolver.
  3. Router finds this method bases on type_name and field_name and calls it for every event in the list. It passes the event itself to it as parameter argument. Optionally it pass user specified parameters if they are present - they are taken from the inside of the event itself. We pass user specified parameters to keep similar UX as with base resolver.
  4. Once all events are processed by decorated method, router combines the output from multiple method calls in a form of a list and send it back to the appsync.

Checklist

If your change doesn't seem to apply, please leave them unchecked.

Is this a breaking change?

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

Acknowledgment

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

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@mploski mploski requested a review from a team as a code owner March 10, 2023 21:45
@mploski mploski requested review from leandrodamascena and removed request for a team March 10, 2023 21:45
@boring-cyborg boring-cyborg bot added dependencies Pull requests that update a dependency file event_handlers tests labels Mar 10, 2023
@pull-request-size pull-request-size bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 10, 2023
@mploski mploski force-pushed the fix/1303-appsync-batch-invoke branch from 1ceecf7 to ccdbbea Compare March 10, 2023 22:06
@codecov-commenter
Copy link

codecov-commenter commented Mar 10, 2023

Codecov Report

Attention: Patch coverage is 99.16667% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 96.35%. Comparing base (e14e768) to head (e14c73f).
Report is 179 commits behind head on develop.

Files Patch % Lines
aws_lambda_powertools/event_handler/appsync.py 98.59% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1998      +/-   ##
===========================================
- Coverage    96.38%   96.35%   -0.03%     
===========================================
  Files          214      219       +5     
  Lines        10030    10334     +304     
  Branches      1846     1928      +82     
===========================================
+ Hits          9667     9957     +290     
- Misses         259      270      +11     
- Partials       104      107       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mploski mploski force-pushed the fix/1303-appsync-batch-invoke branch from ccdbbea to d0fe867 Compare March 10, 2023 22:10
@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 10, 2023
@mploski mploski force-pushed the fix/1303-appsync-batch-invoke branch 2 times, most recently from 72dc94f to 524d054 Compare March 23, 2023 13:38
@mploski mploski force-pushed the fix/1303-appsync-batch-invoke branch from 524d054 to bc45703 Compare March 23, 2023 13:42
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.

Thank you so much for the change, that's much better! Made one comment about favouring composition instead of multiple inheritance (it'll bite us!).

One super helpful change would be to update PR description with a sample UX, so that customers could easily spot what's coming, we could reuse in our docs, and release notes :-)

I'll do a proper review next week

aws_lambda_powertools/event_handler/appsync.py Outdated Show resolved Hide resolved
aws_lambda_powertools/event_handler/appsync.py Outdated Show resolved Hide resolved
aws_lambda_powertools/event_handler/appsync.py Outdated Show resolved Hide resolved
@pull-request-size pull-request-size bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 31, 2023
@mploski mploski force-pushed the fix/1303-appsync-batch-invoke branch 3 times, most recently from efcb664 to 5668cba Compare March 31, 2023 11:20
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Mar 14, 2024
Copy link

sonarcloud bot commented Mar 14, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
2.1% Duplication on New Code

See analysis details on SonarCloud

@heitorlessa
Copy link
Contributor

can I have a quick update on this, please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation event_handlers feature New feature or functionality size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: support BatchInvoke when using AppSyncResolver
8 participants