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

Tech debt: Refactor api_gateway.py file by splitting its code into multiple files. #4194

Open
1 of 2 tasks
leandrodamascena opened this issue Apr 24, 2024 · 4 comments
Open
1 of 2 tasks
Labels
event_handlers tech-debt Technical Debt tasks v3 Features that will be included in Powertools v3.

Comments

@leandrodamascena
Copy link
Contributor

Why is this needed?

Refactor api_gateway.py by splitting its "monolithic" structure with many classes and methods into multiple files for better code organization and maintainability.

Which area does this relate to?

Event Handler - REST API

Suggestion

No response

Acknowledgment

@leandrodamascena leandrodamascena added triage Pending triage from maintainers tech-debt Technical Debt tasks event_handlers v3 Features that will be included in Powertools v3. and removed triage Pending triage from maintainers labels Apr 24, 2024
@leandrodamascena leandrodamascena added this to the Powertools v3 milestone Apr 24, 2024
@leandrodamascena
Copy link
Contributor Author

Hello @heitorlessa, before start working on this issue, I would like to hear your opinion here. The api_gateway.py file is quite complicated to understand now because we have added a lot of things like Middleware, OpenAPI and others, so my suggestion is to split this file into multiple files and keep the export at a high level for the existing items. I'm not sure about some circular dependencies problems, but I think we can make it work.

(aws-lambda-powertools-py3.10) ➜  event_handler git:(split-api-gw-code) ✗ tree
.
├── __init__.py
├── api_gateway.py --------> **Remove this file??**  
├── appsync.py
├── constants.py --------> **Move ProxyEventType and other constants here**  
├── content_types.py
├── cors.py ------> **Move CORSConfig here**
├── exceptions.py
├── middlewares
│   ├── __init__.py
│   ├── base.py --------> **Move MiddlewareFrame here**  
│   ├── openapi_validation.py
│   └── schema_validation.py
├── openapi
│   ├── __init__.py
│   ├── compat.py
│   ├── constants.py
│   ├── dependant.py
│   ├── encoders.py
│   ├── exceptions.py
│   ├── models.py
│   ├── params.py
│   ├── swagger_ui
│   │   ├── __init__.py
│   │   ├── html.py
│   │   ├── oauth2.py
│   │   ├── swagger-ui-bundle.min.js
│   │   └── swagger-ui.min.css
│   └── types.py
├── resolvers
│   ├── api_gateway_http.py ------> **Move APIGatewayHttpResolver here**
│   ├── api_gateway_rest.py ------> **Move APIGatewayRestResolver here**
│   ├── application_load_balancer.py ------> **Move ALBResolver here**
│   ├── base.py ------> **Move ApiGatewayResolver here**
│   ├── bedrock_agent.py ------> **Move BedrockAgentResolver here**
│   ├── lambda_function_url.py ------> **Move LambdaFunctionUrlResolver here**
│   └── vpc_lattice.py ------> **Move VPCLatticeV2Resolver and VPCLatticeResolver here**
├── response.py ------> **Move ResponseBuilder and Response here**
├── router.py ------> **Move Router and Route here**
├── types.py
└── util.py

The __init__.py file could be something like this:

from aws_lambda_powertools.event_handler.appsync import AppSyncResolver
from aws_lambda_powertools.event_handler.resolvers.api_gateway_http import APIGatewayHttpResolver
from aws_lambda_powertools.event_handler.resolvers.api_gateway_rest import APIGatewayRestResolver
from aws_lambda_powertools.event_handler.resolvers.application_load_balancer import ALBResolver
from aws_lambda_powertools.event_handler.resolvers.bedrock_agent import BedrockAgentResolver
from aws_lambda_powertools.event_handler.resolvers.lambda_function_url import (
    LambdaFunctionUrlResolver,
)
from aws_lambda_powertools.event_handler.resolvers.vpc_lattice import VPCLatticeResolver, VPCLatticeV2Resolver

__all__ = [
    "AppSyncResolver",
    "APIGatewayRestResolver",
    "APIGatewayHttpResolver",
    "ALBResolver",
    "ApiGatewayResolver",
    "BedrockAgentResolver",
    "CORSConfig",
    "LambdaFunctionUrlResolver",
    "Response",
    "VPCLatticeResolver",
    "VPCLatticeV2Resolver",
]

What do you think about?

@heitorlessa
Copy link
Contributor

heitorlessa commented May 15, 2024 via email

@leandrodamascena
Copy link
Contributor Author

Whenever you have time, could you share the list of imports that will break for customers using absolute imports with these? It’ll be useful for import hooks (prevent breaking), and to have a better idea whether we go one step further towards modularisation

The short answer for this is: everything importing from api_gateway will break.

from aws_lambda_powertools.event_handler.api_gateway import APIGatewayRestResolver
from aws_lambda_powertools.event_handler.api_gateway import Router
from aws_lambda_powertools.event_handler.api_gateway import .......

We can think about using import hooks to avoid any break in runtime. I just don't know if this break IDE hints/autocompletion.

@heitorlessa
Copy link
Contributor

Assuming import hooks works, I'd be fine with the change :) I'll have more thoughts as the PR gets implemented, for now, what's top of mind for me is:

  • appsync.py should be moved to resolvers, possibly broken too as there's some notable pieces that shouldn't be a single file
  • we're missing a responses subpackage to compartmentalise content_types.py, response.py, and future-proof anything that can reduce boilerplate in building known response formats

In the future, we might expand to modularization, where most of this will be under a core, and resolvers will be separate subpackages entirely. That's one of the reasons import hooks are crucial - both for backwards compatibility and future-proof a new era of highly specialized Event Handlers e.g., CWLogs, EventBridge, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
event_handlers tech-debt Technical Debt tasks v3 Features that will be included in Powertools v3.
Projects
Status: Working on it
Development

No branches or pull requests

2 participants