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(apigateway): update Response class to require status_code only #1560

Merged

Conversation

heitorlessa
Copy link
Contributor

@heitorlessa heitorlessa commented Sep 30, 2022

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
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
Copy link

codecov-commenter commented Sep 30, 2022

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