Skip to content

fix(apigateway): update Response class to require status_code only#1560

Merged
heitorlessa merged 3 commits into
aws-powertools:developfrom
heitorlessa:fix/response-optional-params
Sep 30, 2022
Merged

fix(apigateway): update Response class to require status_code only#1560
heitorlessa merged 3 commits into
aws-powertools:developfrom
heitorlessa:fix/response-optional-params

Conversation

@heitorlessa

@heitorlessa heitorlessa commented Sep 30, 2022

Copy link
Copy Markdown
Contributor

Issue number: #1559

Summary

Changes

Please provide a summary of what's being changed

Honours docstring by making content_type and body parameters as optional since they already support defaults when built.

User experience

Please share what the user experience looks like before and after this change

Before

from aws_lambda_powertools import Logger
from aws_lambda_powertools.event_handler import APIGatewayRestResolver, Response
from aws_lambda_powertools.logging import correlation_paths
from aws_lambda_powertools.utilities.typing import LambdaContext

logger = Logger()
app = APIGatewayRestResolver()


@app.get("/todos")
def get_todos():
    return Response(status_code=204, content_type="application/json", body=None)


@logger.inject_lambda_context(correlation_id_path=correlation_paths.API_GATEWAY_REST)
def lambda_handler(event: dict, context: LambdaContext) -> dict:
    return app.resolve(event, context)

After

from aws_lambda_powertools import Logger
from aws_lambda_powertools.event_handler import APIGatewayRestResolver, Response
from aws_lambda_powertools.logging import correlation_paths
from aws_lambda_powertools.utilities.typing import LambdaContext

logger = Logger()
app = APIGatewayRestResolver()


@app.get("/todos")
def get_todos():
    return Response(status_code=204)


@logger.inject_lambda_context(correlation_id_path=correlation_paths.API_GATEWAY_REST)
def lambda_handler(event: dict, context: LambdaContext) -> dict:
    return app.resolve(event, context)

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.

@pull-request-size pull-request-size Bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 30, 2022
@heitorlessa heitorlessa changed the title fix(apigateway): response class signature to only set status_code required fix(apigateway): update response class signature to only have status_code required Sep 30, 2022
@github-actions

Copy link
Copy Markdown
Contributor

No related issues found. Please ensure there is an open issue related to this change to avoid significant delays or closure.

@github-actions github-actions Bot added do-not-merge need-issue PRs that are missing related issues bug Something isn't working labels Sep 30, 2022
@heitorlessa
heitorlessa marked this pull request as ready for review September 30, 2022 13:28
@heitorlessa
heitorlessa requested a review from a team as a code owner September 30, 2022 13:28
@heitorlessa
heitorlessa requested review from rubenfonseca and removed request for a team September 30, 2022 13:28
@heitorlessa heitorlessa removed do-not-merge need-issue PRs that are missing related issues labels Sep 30, 2022
@codecov-commenter

codecov-commenter commented Sep 30, 2022

Copy link
Copy Markdown

Codecov Report

Base: 99.73% // Head: 99.73% // No change to project coverage 👍

Coverage data is based on head (3134fa4) compared to base (effb910).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1560   +/-   ##
========================================
  Coverage    99.73%   99.73%           
========================================
  Files          124      124           
  Lines         5718     5718           
  Branches       651      651           
========================================
  Hits          5703     5703           
  Misses           8        8           
  Partials         7        7           
Impacted Files Coverage Δ
aws_lambda_powertools/event_handler/api_gateway.py 100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@heitorlessa
heitorlessa requested review from leandrodamascena and removed request for rubenfonseca September 30, 2022 13:29
@heitorlessa heitorlessa changed the title fix(apigateway): update response class signature to only have status_code required fix(apigateway): update response class signature to only require status_code Sep 30, 2022
@heitorlessa heitorlessa changed the title fix(apigateway): update response class signature to only require status_code fix(apigateway): update response class signature to require status_code only Sep 30, 2022
@heitorlessa heitorlessa changed the title fix(apigateway): update response class signature to require status_code only fix(apigateway): update Response class to require status_code only Sep 30, 2022
@heitorlessa
heitorlessa merged commit bb8af11 into aws-powertools:develop Sep 30, 2022
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/S Denotes a PR that changes 10-29 lines, ignoring generated files. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Response class does not honour docstrings on optional parameters

3 participants