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

Update docstring and type hint for lambda_handler #95

Merged
merged 2 commits into from
Jun 5, 2019
Merged

Update docstring and type hint for lambda_handler #95

merged 2 commits into from
Jun 5, 2019

Conversation

nikhilym
Copy link
Contributor

@nikhilym nikhilym commented Jun 4, 2019

Description

Update the type hint for skill_builder's lambda_handler , to reflect the (de) serialization component. Also add docstring information to mention how the lambda_handler is processing the input from AWS Lambda.

Motivation and Context

Fixes #93 . Since sb.lambda_handler is specific to AWS Lambda, the underlying function should only accept parameter types specific to Lambda processing. So changing the type hinting to reflect that. Also mentioning more information on how this is processed in the skill code.

Testing

Generated sphinx docs locally to check the docstrings. Checked the automated documentation in IDE's to reflect the type hints. Since the lambda_handler function is a wrapper function and the actual type hints are on the wrapper, the IDE doesn't show any hints for the lambda_handler function itself. mypy tests on ask-sdk-core also passes through travis.

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed

License

  • I confirm that this pull request can be released under the Apache 2 license

@nikhilym nikhilym requested a review from a team June 4, 2019 19:18
Copy link
Contributor

@pbheemag pbheemag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: serialized to* (in the function description)

@nikhilym nikhilym requested a review from a team June 4, 2019 19:22
@nikhilym
Copy link
Contributor Author

nikhilym commented Jun 4, 2019

@pbheemag thanks for the comment.

nit: serialized to* (in the function description)

However, I don't think I need to add a to here because I mention that the output is the serialized model class.

@nikhilym
Copy link
Contributor Author

nikhilym commented Jun 5, 2019

Travis build is fine. Looks like github integration with travis is facing problems. So going ahead with merging this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

skill_builder.lambda_handler's documentation showing RequestEnvelope as parameter but expects json-string
3 participants