Bug: Revert changes that impacts v3

[leandrodamascena]

Expected Behaviour

Some utilities should work without any extra dependency.

Current Behaviour

The V3 branch does not yet have nox installed to perform dependency testing and some of our changes have created some unexpected cross-dependency breaking changes.

Here is the list of breaking changes detected so far:

1 - We can't import BaseModel outside of TYPE_CHECKING because it will force to bring Pydantic at Runtime level even not using data_validation - https://github.com/aws-powertools/powertools-lambda-python/blob/v3/aws_lambda_powertools/event_handler/openapi/types.py#L7

2 - We can't import ConfigDict because we will force to bring Pydantic event not using data_validation - https://github.com/aws-powertools/powertools-lambda-python/blob/v3/aws_lambda_powertools/event_handler/openapi/constants.py#L1

3 - We need to restore get_header.. and get_querystring.. functions removed by this PR - https://github.com/aws-powertools/powertools-lambda-python/pull/4606/files

4 - We need to add more e2e tests.

Code snippet

No code snippet

Possible Solution

No response

Steps to Reproduce

Install and run

Powertools for AWS Lambda (Python) version

v3

AWS Lambda function runtime

3.11

Packaging format used

PyPi

Debugging logs

No response

[ericbn]

@leandrodamascena, regarding point 1, as far as I can see, code has always been importing BaseModel outside TYPE_CHECKING. This makes sense, because otherwise it would break Pydantic.

Looks like for both points 1 and 2, Pydantic-specific code will need to be under some runtime check depending on each case. For example instead of:

        if isinstance(res, BaseModel):
            return _model_dump(
                res,
                by_alias=True,
                exclude_unset=exclude_unset,
                exclude_defaults=exclude_defaults,
                exclude_none=exclude_none,
            )

we could use a duck-typing-"Look Before You Leap" (LBYL) style like:

        if hasattr(res, "model_dump"):
            return _model_dump(
                res,
                by_alias=True,
                exclude_unset=exclude_unset,
                exclude_defaults=exclude_defaults,
                exclude_none=exclude_none,
            )

or an arguably preferred (because under the hood, hasattr is actually checking for an AttributeError exception being raised by getattr) "Easier to Ask Forgiveness Than Permission" (EAFP) style like:

        try:
            return _model_dump(
                res,
                by_alias=True,
                exclude_unset=exclude_unset,
                exclude_defaults=exclude_defaults,
                exclude_none=exclude_none,
            )
        except AttributeError:
            ...

I'd say it goes on depending on each code that should only optionally work with Pydantic, only if it's available.

[leandrodamascena]

@leandrodamascena, regarding point 1, as far as I can see, code has always been importing BaseModel outside TYPE_CHECKING. This makes sense, because otherwise it would break Pydantic.

I don't think @ericbn! We are importing this inside the TYPE_CHECKING and using quotes to reference it and avoid forward reference.

https://github.com/aws-powertools/powertools-lambda-python/blob/develop/aws_lambda_powertools/event_handler/openapi/types.py#L8

if TYPE_CHECKING:
    from pydantic import BaseModel  # noqa: F401

CacheKey = Optional[Callable[..., Any]]
IncEx = Union[Set[int], Set[str], Dict[int, Any], Dict[str, Any]]
ModelNameMap = Dict[Union[Type["BaseModel"], Type[Enum]], str]
TypeModelOrEnum = Union[Type["BaseModel"], Type[Enum]]
UnionType = getattr(types, "UnionType", Union)

[ericbn]

Oi @leandrodamascena. Sorry, I now realize there are at least 3 different scenarios and I was not properly distinguishing between them:

1. pydantic imports used only for type checking: I was not aware of this. I see this was broken in commit 689072f (#4990) and you fixed it. My bad I've missed this before. I've double checked that this was the only regression introduced as part of the recent changes regarding #4607, and I've created #5088 as I was double checking this.
2. pydantic optionally used only when the package is installed, that should not try to import the package otherwise: I'm not familiar with this scenario, but I see some logic under different checks like "pydantic" in sys.modules or PYDANTIC_V2.
3. pydantic BaseModel classes being defined, which require the pydantic imports and the types used in the BaseModel definitions to be outside TYPE_CHECKING: We've tried to make sure we didn't break this as part of the changes regarding #4607.

TL;DR My last comment above is a mess mixing up independent scenarios where pydantic is being used in different ways. :- )

[leandrodamascena]

TL;DR My last comment above is a mess mixing up independent scenarios where pydantic is being used in different ways. :- )

This is completely fine and important because you define exactly how we use Pydantic in different scenarios. I appreciate that.

2. pydantic optionally used only when the package is installed, that should not try to import the package otherwise: I'm not familiar with this scenario, but I see some logic under different checks like "pydantic" in sys.modules or PYDANTIC_V2.

Wow, this made me search for PYDANTIC_V2 in the v3 branch and I found a reference that we should not have. I'll fix that. Thanks for bringing this to light 😄

[leandrodamascena]

Closing this as completed.

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

This issue is now closed. Please be mindful that future comments are hard for our team to see.

If you need more assistance, please either tag a team member or open a new issue that references this one.

If you wish to keep having a conversation with other community members under this issue feel free to do so.

[github-actions]

This is now released under 3.0.0 version!