-
Notifications
You must be signed in to change notification settings - Fork 406
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(batch): inject lambda_context if record handler signature accepts it #1561
feat(batch): inject lambda_context if record handler signature accepts it #1561
Conversation
Codecov ReportBase: 99.73% // Head: 99.73% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #1561 +/- ##
========================================
Coverage 99.73% 99.73%
========================================
Files 124 124
Lines 5718 5727 +9
Branches 651 653 +2
========================================
+ Hits 5703 5712 +9
Misses 8 8
Partials 7 7
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
@@ -94,7 +98,7 @@ def __enter__(self): | |||
def __exit__(self, exception_type, exception_value, traceback): | |||
self._clean() | |||
|
|||
def __call__(self, records: List[dict], handler: Callable): | |||
def __call__(self, records: List[dict], handler: Callable, lambda_context: Optional[LambdaContext] = None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note to self: take the opportunity to make records
typing more specific. Need to check with Mypy accepts it due to liskov substitution principle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since I can access record properties in my Lambda code, I'm not sure this is a real concern for now. Maybe in the future it's worth spending some time on it?
But yes, I love static and strong typing ❤️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @heitorlessa! Another PR without errors and changes, you are a wizard! 🧙
@@ -94,7 +98,7 @@ def __enter__(self): | |||
def __exit__(self, exception_type, exception_value, traceback): | |||
self._clean() | |||
|
|||
def __call__(self, records: List[dict], handler: Callable): | |||
def __call__(self, records: List[dict], handler: Callable, lambda_context: Optional[LambdaContext] = None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since I can access record properties in my Lambda code, I'm not sure this is a real concern for now. Maybe in the future it's worth spending some time on it?
But yes, I love static and strong typing ❤️
self._handler_accepts_lambda_context = False | ||
else: | ||
self.lambda_context = lambda_context | ||
self._handler_accepts_lambda_context = "lambda_context" in inspect.signature(self.handler).parameters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to update the documentation to make it clear that the method signature must have a lambda_context
parameter otherwise it won't work. The first time I tried to add a context
parameter and I couldn't access LambdaContext properties LOL 😵💫
Mentioning #1369 not to forget when refactoring documentation.
Issue number: #1242
Summary
Changes
This PR allows customers to receive a Lambda Context to help calculate how much time left before a request times out, among execution environment metadata.
User experience
Record handlers can now update their signature with an additional parameter named
lambda_context
.BEFORE
AFTER
@batch_processor decorator
When using
@batch_processor
decorator, we'll automatically inject the Lambda context.batch processor context manager
When using the context manager, customers will need to pass the Lambda context as an additional parameter:
Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change?
RFC issue number:
Checklist:
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.