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

fix(event_handler): add tests for PEP 563 compatibility with OpenAPI #5886

Merged
merged 1 commit into from
Jan 20, 2025

Conversation

leandrodamascena
Copy link
Contributor

Issue number: #5098

Summary

Changes

This pull request add tests to make sure that OpenAPI is working when data validation feature of event resolvers and the use of from __future__ import annotations, as introduced by PEP 563 (Postponed Evaluation of Annotations).

Some additional information

  1. This PR fix a bug when getting types from __globals__.
  2. Runtime Optimization: There was a need to import annotations to ensure type definitions are ignored at runtime, aligning with PEP 563's goals.
  3. We advice to use Pydantic 2.10+ at least.

While this PR make sure this is working, I'm working in additional PR's to make sure Powertools is full compatible with Pydantic 2.10+.

User experience

Before this PR the following code could fail with:

pydantic.errors.PydanticUserError: `TypeAdapter[typing.Annotated[ForwardRef('Input'), Query(PydanticUndefined)]]` is not fully defined; you should define `typing.Annotated[ForwardRef('Input'), Query(PydanticUndefined)]` and all referenced types, then call `.rebuild()` on the instance.
from __future__ import annotations

import json
from typing import TYPE_CHECKING

from aws_lambda_powertools.event_handler import APIGatewayRestResolver
from pydantic import BaseModel

if TYPE_CHECKING:
    # Actual use case imports boto stubs
    # These are giant pyi files that shouldn't be shipped at runtime
    from typing import Iterable


class Input(BaseModel):
    email: str

class Output(BaseModel):
    response: str

app = APIGatewayRestResolver(enable_validation=True)


@app.post("/hello")
def hello(event: Input) -> Output:
    return Output(response=f"Hello {event.email}")


def func(a: Iterable[int]): ...


event = {
    "path": "/hello",
    "httpMethod": "POST",
    "requestContext": {
        "requestId": "227b78aa-779d-47d4-a48e-ce62120393b8",
    },
    "body": json.dumps(
        {
            "email": "[email protected]",
        }
    ),
}

print(app.resolve(event, {}))

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.

@leandrodamascena leandrodamascena requested a review from a team as a code owner January 20, 2025 11:43
@boring-cyborg boring-cyborg bot added the tests label Jan 20, 2025
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 20, 2025
Copy link

codecov bot commented Jan 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.19%. Comparing base (465afe5) to head (3b4b989).
Report is 1 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #5886      +/-   ##
===========================================
+ Coverage    96.17%   96.19%   +0.02%     
===========================================
  Files          232      232              
  Lines        10941    10941              
  Branches      2023     2023              
===========================================
+ Hits         10522    10525       +3     
+ Misses         329      327       -2     
+ Partials        90       89       -1     

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

@leandrodamascena
Copy link
Contributor Author

leandrodamascena commented Jan 20, 2025

e2e is green: https://github.com/aws-powertools/powertools-lambda-python/actions/runs/12867394165/job/35873665498

This error is not relevant at this time and happened randomly in Python 3.13:

FAILED tests/e2e/event_handler/test_cors.py::test_alb_cors_with_correct_origin - requests.exceptions.ConnectionError: HTTPConnectionPool(host='testv3-albae-vnokvaixfzx1-402307046.us-east-1.elb.amazonaws.com', port=80): Max retries exceeded with url: /todos (Caused by NewConnectionError('<urllib3.connection.HTTPConnection object at 0x7f685640e3f0>: Failed to establish a new connection: [Errno 111] Connection refused'))

@leandrodamascena leandrodamascena changed the title fix(event_handler): resolve PEP563 compatibility with OpenAPI + tests fix(event_handler): add tests for PEP563 compatibility with OpenAP Jan 20, 2025
@leandrodamascena leandrodamascena changed the title fix(event_handler): add tests for PEP563 compatibility with OpenAP fix(event_handler): add tests for PEP 563 compatibility with OpenAPI Jan 20, 2025
@leandrodamascena leandrodamascena merged commit fea7687 into develop Jan 20, 2025
26 of 27 checks passed
@leandrodamascena leandrodamascena deleted the fix/openapi-forward-ref branch January 20, 2025 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: APIGatewayRestResolver(enable_validation=True) incompatible with from __future__ import annotations
2 participants