Skip to content

Conversation

michaelbrewer
Copy link
Contributor

@michaelbrewer michaelbrewer commented Sep 21, 2021

Issue #, if available:

Description of changes:

Current implementation will just build a message from the JsonSchemaValueException like:

'Failed schema validation. Error: data must contain [\'message\', \'username\'] properties, Path: [\'data\'], Data: {\'message\': \'hello_world\'}'

Which is not presentable and missing some of the data elements. This change updates SchemaValidationError to include more data elements from JsonSchemaValueException

New fields are:

  • validation_message : Containing human-readable information what is wrong (e.g. data.property[index] must be smaller than or equal to 42)
  • name : name of a path in the data structure (e.g. data.property[index])
  • path: path as an array in the data structure (e.g. ['data', 'property', 'index']),
  • value : The invalid value
  • definition : The full rule definition (e.g. 42)
  • rule : - rule which the data is breaking (e.g. maximum)
  • rule_definition : Any, optional The specific rule definition (e.g. 42)

Checklist

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

@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 21, 2021
Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

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

Two small changes: 1/ Test to ensure we don't break the previous behaviour, and 2/ Either docs or an example of how to leverage the new attributes to make it presentable

@boring-cyborg boring-cyborg bot added the tests label Sep 22, 2021
@michaelbrewer
Copy link
Contributor Author

@heitorlessa not sure what to change

@codecov-commenter
Copy link

codecov-commenter commented Sep 22, 2021

Codecov Report

Merging #686 (4346ae0) into develop (505891d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #686   +/-   ##
========================================
  Coverage    99.97%   99.97%           
========================================
  Files          116      116           
  Lines         4855     4866   +11     
  Branches       267      267           
========================================
+ Hits          4854     4865   +11     
  Partials         1        1           
Impacted Files Coverage Δ
aws_lambda_powertools/utilities/validation/base.py 100.00% <100.00%> (ø)
...mbda_powertools/utilities/validation/exceptions.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 505891d...4346ae0. Read the comment docs.

Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

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

Thanks a lot, Mike!

@heitorlessa heitorlessa added feature New feature or functionality area/validator labels Sep 23, 2021
@heitorlessa heitorlessa merged commit e24eb02 into aws-powertools:develop Sep 23, 2021
@heitorlessa
Copy link
Contributor

@heitorlessa not sure what to change

My internet didn't download all details and I couldn't see the tests -- this time I can, just merged, sorry for the confusion

heitorlessa added a commit to heitorlessa/aws-lambda-powertools-python that referenced this pull request Sep 28, 2021
…tools-python into develop

* 'develop' of https://github.com/awslabs/aws-lambda-powertools-python:
  docs(event-handler): document catch-all routes (aws-powertools#705)
  chore: add python 3.9 support
  docs: add team behind it and email
  ISSUE-693: Use ExpressionAttributeNames in _put_record (aws-powertools#697)
  feat(validator): include missing data elements from a validation error (aws-powertools#686)
  chore(deps-dev): bump mkdocs-material from 7.2.8 to 7.3.0 (aws-powertools#695)
  chore(deps-dev): bump mkdocs-material from 7.2.6 to 7.2.8 (aws-powertools#682)
  chore(deps-dev): bump flake8-bugbear from 21.4.3 to 21.9.1 (aws-powertools#676)
  chore(deps): bump boto3 from 1.18.38 to 1.18.41 (aws-powertools#677)
  chore(deps-dev): bump radon from 4.5.2 to 5.1.0 (aws-powertools#673)
  chore(deps): bump boto3 from 1.18.32 to 1.18.38 (aws-powertools#671)
  refactor(data-classes): clean up internal logic for APIGatewayAuthorizerResponse (aws-powertools#643)
  fix(data-classes): use correct asdict funciton (aws-powertools#666)
  chore(deps-dev): bump xenon from 0.7.3 to 0.8.0 (aws-powertools#669)
  chore: bump to 1.20.2
  fix: Fix issue with strip_prefixes (aws-powertools#647)
  chore(deps-dev): bump mkdocs-material from 7.2.4 to 7.2.6 (aws-powertools#665)
  chore(deps): bump boto3 from 1.18.26 to 1.18.32 (aws-powertools#663)
  chore(deps-dev): bump pytest from 6.2.4 to 6.2.5 (aws-powertools#662)
  chore(license): Add THIRD-PARTY-LICENSES (aws-powertools#641)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or functionality size/M Denotes a PR that changes 30-99 lines, ignoring generated files. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants