powertools-lambda-python icon indicating copy to clipboard operation
powertools-lambda-python copied to clipboard

Feature request: Return empty Dict or List in Event Source Data Classes instead of None

Open ericbn opened this issue 2 years ago • 6 comments

Use case

Arguably, it might be easier for users if an empty Dict is returned instead of None for path_parameters in APIGatewayProxyEventV2, for example.

Instead of:

@event_source(data_class=APIGatewayProxyEventV2)
def lambda_handler(event: APIGatewayProxyEventV2, context) -> None:
    parameters = event.path_parameters or {}
    handle(parameters["id"])

it would become:

@event_source(data_class=APIGatewayProxyEventV2)
def lambda_handler(event: APIGatewayProxyEventV2, context) -> None:
    handle(event.path_parameters["id"])

Ultimately this approach could be followed for any return value that is either an Optional[Dict] or an Optional[List], returning an empty Dict or List instead of None in all cases:

  • utilities/data_classes/api_gateway_proxy_event.py
    • APIGatewayEventAuthorizer
      • def claims(self) -> Dict[str, Any]
      • def scopes(self) -> List[str]
    • APIGatewayProxyEvent
      • def multi_value_query_string_parameters(self) -> Dict[str, List[str]]
      • def path_parameters(self) -> Dict[str, str]
      • def stage_variables(self) -> Dict[str, str]
    • RequestContextV2AuthorizerIam
      • def cognito_amr(self) -> List[str]
    • RequestContextV2Authorizer
      • def jwt_claim(self) -> Dict[str, Any]
      • def jwt_scopes(self) -> List[str]
      • def get_lambda(self) -> Dict[str, Any]
    • APIGatewayProxyEventV2
      • def cookies(self) -> List[str]
      • def path_parameters(self) -> Dict[str, str]
      • def stage_variables(self) -> Dict[str, str]

and so on...

The bottom line is: Does it matter for users to differentiate between None and an empty Dict or List for any Event Source Data Classe property? If it does for a particular one, it an exception to a rule or the rule?

This change would be backwards compatible with the previous code.

This approach was already implemented in only one place so far, as far as I could check:

  • utilities/data_classes/common.py
    • BaseProxyEvent
      • def headers(self) -> Dict[str, str]

Solution/User Experience

Solution can be the same as for the headers in the BaseProxyEvent:

https://github.com/aws-powertools/powertools-lambda-python/blob/1905e4cb022b5547d75064a643358c2343977be5/aws_lambda_powertools/utilities/data_classes/common.py#L98-L100

Alternative solutions

No response

Acknowledgment

ericbn avatar Jun 28 '23 20:06 ericbn

Hi @ericbn thank you for sending this proposal! Two notes:

  1. Do you agree that code that backwards compatibility can be ensured since existing code that relies on None as a valid value will still function correctly?
  2. Providing specific use cases and examples where this feature significantly improves the developer experience would strengthen the proposal.

It seems to be this would be a big breaking change anyways. So we might have to open an RFC to discuss this with the larger community and consider the breaking change for v3.

rubenfonseca avatar Jun 29 '23 09:06 rubenfonseca

Hi. Totally agree this is a breaking change for existing code that relies on None as a valid value. Good point. I see the change in the headers method in BaseProxyEvent was done exactly to keep backward compatibility.

I propose this change is favor of an API with multiple methods like get_query_string_value and get_header_value in BaseProxyEvent.

https://github.com/aws-powertools/powertools-lambda-python/blob/2c4420731f2e36f4e0ef3eb11c4a5e13296070c5/aws_lambda_powertools/utilities/data_classes/common.py#L139-L157

https://github.com/aws-powertools/powertools-lambda-python/blob/2c4420731f2e36f4e0ef3eb11c4a5e13296070c5/aws_lambda_powertools/utilities/data_classes/common.py#L159-L186

Pros:

  • No need to create extra methods in the data classes.
  • User can use the List and Dict API directly, for example to choose between event.query_string_parameters["foo"] or event.query_string_parameters.get("foo") or event.query_string_parameters.get("foo", "default_foo") in case the key is required or not, or to use other functions like first(event.cookies).

Cons(?):

  • As the List and Dict APIs are used directly, a parameter like case_sensitive in get_header_value would have to be replaced by returning a custom CaseInsensitiveDict.

ericbn avatar Jul 11 '23 16:07 ericbn

Hello @ericbn! We've updated our roadmap and included this feature request. We are planning to release this feature in Powertools v3.

Roadmap for v3: https://docs.powertools.aws.dev/lambda/python/latest/roadmap/#v3

leandrodamascena avatar Aug 17 '23 21:08 leandrodamascena

Hi @leandrodamascena, cool! I'll be happy to provide a PR if that sounds good. Thanks for considering and accepting the feature request!

ericbn avatar Aug 20 '23 00:08 ericbn

Hello @ericbn! We definitely want you to submit a PR and contribute to the project, this idea of yours brings more consistency to the use of a data class. I just would like to ask is that you wait until we create the Powertools v3 RFC and have the plan defined. If you send the PR now it will take a long time without updating and it will be hard to merge in the future.

I created a new label v3 and added it to this issue, so it's easier for us to track what we need to include in the RFC for Powertools v3.

Thank you.

leandrodamascena avatar Aug 20 '23 10:08 leandrodamascena

Sure, I’ll wait. Makes perfect sense. Looking forward for my first contribution with a PR to the project!

ericbn avatar Aug 23 '23 12:08 ericbn

Took a long time (stability/backwards compatibility) but we've started implementing for v3: https://github.com/aws-powertools/powertools-lambda-python/issues/4189

heitorlessa avatar Jun 10 '24 08:06 heitorlessa

Oi @heitorlessa. I'll be happy to work on this as a contribution to the project.

ericbn avatar Jun 10 '24 13:06 ericbn

Closed via https://github.com/aws-powertools/powertools-lambda-python/pull/4606

leandrodamascena avatar Jul 29 '24 21:07 leandrodamascena

⚠️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[bot] avatar Jul 29 '24 21:07 github-actions[bot]