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

docs: new parser utility #192

Merged
merged 29 commits into from
Oct 25, 2020
Merged

Conversation

heitorlessa
Copy link
Contributor

@heitorlessa heitorlessa commented Oct 14, 2020

Issue #, if available: #118

Description of changes:

Documentation for upcoming advanced parser.

Rendered doc: https://github.com/heitorlessa/aws-lambda-powertools-python/blob/docs/parser/docs/content/utilities/parser.mdx

Checklist

  • Meet tenets criteria
  • Update tests
  • Update docs
  • PR title follows conventional commit semantics
  • Document key features
  • Document how to install additional dependencies (pydantic, typing_extension)
  • Document event_parser decorator
  • Document parse standalone function
  • Document models vs envelopes terminology
  • Document built-in envelopes
  • Document inheriting models to access all their properties e.g. EventBridgeModel
  • Document how to use root_validator and validator
  • Document exception handling
  • Document cold start impact
  • Review by Ran (utility author)
  • If possible, get a review by Koudai
  • revert: Use Pydantic's ValidationError as opposed to ours given its utility functions json(), errors(), etc. Original implementation also provided a better experience for those migrating existing Pydantic models and validations, which I can only see why not.

Breaking change checklist

RFC issue #:

  • Migration process documented
  • Implement warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

* develop: (23 commits)
  improv: rename schema to model as per Tom's review
  improv: rename to event_parser as per Tom's review
  Update aws_lambda_powertools/utilities/parser/pydantic.py
  chore: remove flake8 polyfill as explicit dep
  chore: explicit DynamoDB Stream schema naming
  improv: expose all pydantic imports
  improv: adjust high level imports
  improv: add docstrings; event internal param renamed
  improv: propagate exception to parser
  chore: lint
  feat: add standalone parse function
  chore: kwarg over arg to ease refactoring
  fix: code inspect issues
  improv: simplify base envelope; increase test cov
  improv: test parser
  fix: unnecessary return; better error handling
  Update README.md (#190)
  improv: raise own exception; remove duplicates
  fix: snake_case
  improv: envelope structure & import
  ...
@heitorlessa heitorlessa added the documentation Improvements or additions to documentation label Oct 14, 2020
@heitorlessa heitorlessa marked this pull request as draft October 14, 2020 19:43
@codecov-io
Copy link

codecov-io commented Oct 20, 2020

Codecov Report

Merging #192 into develop will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #192   +/-   ##
========================================
  Coverage    99.87%   99.87%           
========================================
  Files           65       66    +1     
  Lines         2451     2457    +6     
  Branches       108      107    -1     
========================================
+ Hits          2448     2454    +6     
  Misses           3        3           
Impacted Files Coverage Δ
...s_lambda_powertools/utilities/parser/exceptions.py 100.00% <ø> (ø)
aws_lambda_powertools/utilities/parser/__init__.py 100.00% <100.00%> (ø)
..._powertools/utilities/parser/envelopes/__init__.py 100.00% <100.00%> (ø)
...mbda_powertools/utilities/parser/envelopes/base.py 100.00% <100.00%> (ø)
..._powertools/utilities/parser/envelopes/dynamodb.py 100.00% <100.00%> (ø)
...ertools/utilities/parser/envelopes/event_bridge.py 100.00% <100.00%> (ø)
...ambda_powertools/utilities/parser/envelopes/sqs.py 100.00% <100.00%> (ø)
aws_lambda_powertools/utilities/parser/parser.py 100.00% <100.00%> (ø)
aws_lambda_powertools/utilities/parser/types.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6a5889b...3750d55. Read the comment docs.

@heitorlessa
Copy link
Contributor Author

hey @risenberg-cyberark @koxudaxi - Would appreciate if you could review the new parser's docs before we release it.

Parser is a thin wrapper on top of Pydantic

* develop:
  fix: high and security peer dependency vulnerabilities
  fix: change to Yarn to support manual resolutions
  docs: use yarn's resolution to fix incompatible dependency
  build(deps): bump object-path from 0.11.4 to 0.11.5 in /docs
Copy link

@koxudaxi koxudaxi left a comment

Choose a reason for hiding this comment

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

@heitorlessa
Thank you for requesting a review.
I wrote some comments.
I think TypeVar can support that users decide types of models.
I guess _parse and parse methods should have TypeVar arguments for Models.

aws_lambda_powertools/utilities/parser/envelopes/sqs.py Outdated Show resolved Hide resolved
aws_lambda_powertools/utilities/parser/envelopes/sqs.py Outdated Show resolved Hide resolved
aws_lambda_powertools/utilities/parser/parser.py Outdated Show resolved Hide resolved
@heitorlessa
Copy link
Contributor Author

heitorlessa commented Oct 24, 2020

@heitorlessa

Thank you for requesting a review.

I wrote some comments.

I think TypeVar can support that users decide types of models.

I guess _parse and parse methods should have TypeVar arguments for Models.

All great comments; I'll reply to them but first a question @koxudaxi:

What's the value in using parser with "str" as a model?

If nothing will be parsed, why not use the data class utility we provide?

We had support for str in the original implementation by Ran but I struggled to see the value if you know the value will be just a str - I might me missing something hence the question

Thanks a lot everyone

@heitorlessa
Copy link
Contributor Author

All addressed @koxudaxi and I've just learned we don't need the additional Type[Model], Model does it just fine according to PyCharm Code inspection feature.

@koxudaxi
Copy link

koxudaxi commented Oct 24, 2020

All addressed @koxudaxi and I've just learned we don't need the additional Type[Model], Model does it just fine according to PyCharm Code inspection feature.

@heitorlessa
By the way, I'm developing a PyCharm plugin of Pydantic. https://github.com/koxudaxi/pydantic-pycharm-plugin
About the plugin is written in an official pydantic document too:smile_cat:

Signed-off-by: heitorlessa <lessa@amazon.co.uk>
Signed-off-by: heitorlessa <lessa@amazon.co.uk>
Signed-off-by: heitorlessa <lessa@amazon.co.uk>
Signed-off-by: heitorlessa <lessa@amazon.co.uk>
@heitorlessa
Copy link
Contributor Author

Thanks a lot @risenberg-cyberark and @koxudaxi for the review - I'm merging this now with all feedbacks being addressed, and will work on the 1.7.0 release tomorrow by EOD to include this publicly ;)

@heitorlessa heitorlessa merged commit b25b2aa into aws-powertools:develop Oct 25, 2020
@heitorlessa heitorlessa deleted the docs/parser branch October 25, 2020 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants