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(event_source): fix type hint #4165

Conversation

robmoss2k
Copy link

@robmoss2k robmoss2k commented Apr 19, 2024

Issue number: #3568

Summary

Changes

Add the Any return type to the event_source function definition. The handler argument already specifies that it will return Any and event_source returns whatever handler returns.

User experience

mypy --strict won't throw error: Untyped decorator makes function "handler" untyped [misc] any longer.

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.

@robmoss2k robmoss2k requested a review from a team as a code owner April 19, 2024 12:18
Copy link

boring-cyborg bot commented Apr 19, 2024

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.
In the meantime, check out the #python channel on our Powertools for AWS Lambda Discord: Invite link

@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 19, 2024
@robmoss2k
Copy link
Author

Fixes #3568.

@leandrodamascena leandrodamascena changed the title improv: Fix type hint in event_source.py refactor(event_source): fix type hint Apr 19, 2024
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.16%. Comparing base (e14e768) to head (a3960b3).
Report is 326 commits behind head on develop.

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

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4165      +/-   ##
===========================================
- Coverage    96.38%   96.16%   -0.22%     
===========================================
  Files          214      218       +4     
  Lines        10030    10459     +429     
  Branches      1846     1934      +88     
===========================================
+ Hits          9667    10058     +391     
- Misses         259      283      +24     
- Partials       104      118      +14     

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

@leandrodamascena
Copy link
Contributor

Hello @robmoss2k! Thanks for submitting this PR and putting effort to solve this problem.

Can you export the libs that are installed in your virtual environment and how you are running the tests? I'm still having problems here with strict check in mypy.

My tests applying this fix:

requirements.txt

aws-lambda-powertools==2.36.0
aws-xray-sdk==2.12.1
boto3==1.34.10
botocore==1.34.10
fastjsonschema==2.19.1
jmespath==0.10.0
mypy==1.8.0
mypy-extensions==1.0.0
python-dateutil==2.8.2
s3transfer==0.10.0
six==1.16.0
tomli==2.0.1
typing_extensions==4.9.0
urllib3==1.26.18
wrapt==1.16.0

main.py

from typing import Any
from aws_lambda_powertools.utilities.data_classes import SQSEvent, event_source
from aws_lambda_powertools.utilities.typing import LambdaContext

@event_source(data_class=SQSEvent)
def handler(event: SQSEvent, _context: LambdaContext) -> dict[str, Any]:
    return {}

running mypy

(.venv) ➜  leo mypy -V
mypy 1.8.0 (compiled: yes)
(.venv) ➜  leo mypy --strict main.py
main.py:5: error: Untyped decorator makes function "handler" untyped  [misc]
Found 1 error in 1 file (checked 1 source file)

Thanks and please let me know if I'm missing something.

@boring-cyborg boring-cyborg bot added the middleware_factory Middleware factory utility label Apr 23, 2024
@pull-request-size pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 23, 2024
Copy link

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

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

See analysis details on SonarCloud

@rubenfonseca
Copy link
Contributor

It seems this change is breaking a lot of things on our CI

@robmoss2k
Copy link
Author

It seems this change is breaking a lot of things on our CI

It's not finished. I need a solid block of time to spend on it. Don't approve it before it's finished. I will get back to it. Feels like I've bitten off more than I can chew... But I don't like to lose.

@heitorlessa
Copy link
Contributor

No worries, it's a hard task, as not everything can be made into strict typing -- for the decorator factory, the new ParamSpec type will be of great help.

@leandrodamascena
Copy link
Contributor

leandrodamascena commented Jul 9, 2024

Hello @robmoss2k! Thank you for putting in the effort here and helping us make the project more strict/strongly typed, which is a big challenge, we really appreciate your effort. But we are closing this PR as not planned for now.

We are about to release Powertools v3 and we will not prioritize that at the moment as we will put v2 in maintenance mode soon. Another thing is that refactoring method signatures and introducing changes to such important utilities can create some unexpected side effects, which we don't want at this point.

For Powertools v4, we plan to refactor the modules and make the project more strict typed.

Again, thanks for your time here and feel free to reopen if you want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement middleware_factory Middleware factory utility size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Static typing: Untyped event_source decorator with mypy --strict
5 participants