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.

Sign up for GitHub

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

Jump to bottom

improv: include example tests in make tests #63

Merged
merged 6 commits into from
Jun 7, 2020
Merged

improv: include example tests in make tests #63

merged 6 commits into from
Jun 7, 2020

Conversation

to-mc
Copy link
Contributor

@to-mc to-mc commented Jun 7, 2020

Issue #, if available:

Description of changes:

Update pytest config to run example tests. Update tests in example/tests to specify env vars as fixture without requiring user to pass them at runtime.

Breaking change checklist

Ignore if it's not a breaking change

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.

@codecov-commenter
Copy link

codecov-commenter commented Jun 7, 2020 •
edited
Loading

Codecov Report

Merging #63 into develop will increase coverage by 0.60%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           develop       #63      +/-   ##
============================================
+ Coverage    99.39%   100.00%   +0.60%     
============================================
  Files           15        15              
  Lines          496       496              
  Branches        45        45              
============================================
+ Hits           493       496       +3     
+ Misses           3         0       -3
Impacted Files Coverage Δ
aws_lambda_powertools/tracing/extensions.py 100.00% <0.00%> (+75.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 158fd57...beb8c51. Read the comment docs.

@to-mc to-mc marked this pull request as draft June 7, 2020 13:14
@to-mc to-mc marked this pull request as ready for review June 7, 2020 13:23
@to-mc to-mc requested a review from heitorlessa June 7, 2020 13:53
@heitorlessa
Copy link
Contributor

Two quick questions as everything else look great.

  1. For those trying to test/deploy the example directly, do we need to update the README given these changes?

  2. Could you add the POWERTOOLS_METRICS_NAMESPACE env in SAM back?

I've run through all steps after checking out this PR and am hitting the namespace issue (template.yaml)

sam local invoke HelloWorldFunction --event events/event.json
2020-06-07 14:19:16,866 aws_lambda_powertools.metrics.base [DEBUG] Serializing...
2020-06-07 14:19:16,866 aws_lambda_powertools.metrics.base [DEBUG] Validating serialized metrics against CloudWatch EMF schema
[ERROR] SchemaValidationError: Invalid format. Error: data._aws.CloudWatchMetrics[0].Namespace must be string, Invalid item: data._aws.CloudWatchMetrics[0].Namespace
Traceback (most recent call last):
  File "/var/task/aws_lambda_powertools/metrics/metrics.py", line 110, in decorate
    metrics = self.serialize_metric_set()
  File "/var/task/aws_lambda_powertools/metrics/base.py", line 186, in serialize_metric_set
    raise SchemaValidationError(message)

heitorlessa
heitorlessa suggested changes Jun 7, 2020
View reviewed changes
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.

Need to re-add POWERTOOLS_METRICS_NAMESPACE back to SAM template as it fails when it runs locally

@to-mc
Copy link
Contributor Author

to-mc commented Jun 7, 2020

Need to re-add POWERTOOLS_METRICS_NAMESPACE back to SAM template as it fails when it runs locally

I added this env var back in PR #62. I just merged develop into this branch so you should see it now.

@to-mc
Copy link
Contributor Author

to-mc commented Jun 7, 2020

Two quick questions as everything else look great.

  1. For those trying to test/deploy the example directly, do we need to update the README given these changes?
  2. Could you add the POWERTOOLS_METRICS_NAMESPACE env in SAM back?

I've run through all steps after checking out this PR and am hitting the namespace issue (template.yaml)

sam local invoke HelloWorldFunction --event events/event.json
2020-06-07 14:19:16,866 aws_lambda_powertools.metrics.base [DEBUG] Serializing...
2020-06-07 14:19:16,866 aws_lambda_powertools.metrics.base [DEBUG] Validating serialized metrics against CloudWatch EMF schema
[ERROR] SchemaValidationError: Invalid format. Error: data._aws.CloudWatchMetrics[0].Namespace must be string, Invalid item: data._aws.CloudWatchMetrics[0].Namespace
Traceback (most recent call last):
  File "/var/task/aws_lambda_powertools/metrics/metrics.py", line 110, in decorate
    metrics = self.serialize_metric_set()
  File "/var/task/aws_lambda_powertools/metrics/base.py", line 186, in serialize_metric_set
    raise SchemaValidationError(message)

README is updated already - please let me know if the steps don't work for you.

@to-mc to-mc requested a review from heitorlessa June 7, 2020 14:35
@heitorlessa
Copy link
Contributor

Just pulled, and it's all working now ;) Merging!

@heitorlessa heitorlessa changed the title chore: include example tests in make tests improv: include example tests in make tests Jun 7, 2020
@heitorlessa heitorlessa merged commit 993e8e5 into develop Jun 7, 2020
@heitorlessa heitorlessa deleted the chore/run_example_tests branch June 7, 2020 15:06
heitorlessa added a commit that referenced this pull request Jun 7, 2020
@heitorlessa
Merge branch 'develop' into feat/precommit-hooks
11463fe 
* develop:
  improv: include example tests in `make tests` (#63)
  chore: rename Makefile target docs-dev to docs-local (#65)
  improv: better namespace/dimension handling for Metrics (#62)
heitorlessa added a commit that referenced this pull request Jun 10, 2020
@heitorlessa
merge latest stable fixes from develop branch
d87f82b 
* develop: (31 commits)
  docs: fix contrast on highlighted code text (#73)
  feat: improve error handling for log_metrics decorator (#71)
  chore(deps): bump graphql-playground-html from 1.6.19 to 1.6.25 in /docs
  feat: add high level imports (#70)
  fix: correct env var name for publish to pypi test (#69)
  chore: version bump (#68)
  feat: add capture_cold_start_metric for log_metrics (#67)
  chore(deps): bump websocket-extensions from 0.1.3 to 0.1.4 in /docs (#66)
  feat: automate publishing to pypi (#58)
  feat: add pre-commit hooks (#64)
  improv: include example tests in `make tests` (#63)
  chore: rename Makefile target docs-dev to docs-local (#65)
  improv: better namespace/dimension handling for Metrics (#62)
  docs: build on master only
  chore: correct docstring for log_metrics
  chore: fix typo in metrics doc
  chore: Correct test comment
  chore: remove unused import
  chore: formatting
  feat: update Metrics interface to resemble tracer & logger: use "service" as its namespace.
  ...
heitorlessa referenced this pull request in heitorlessa/aws-lambda-powertools-python Jun 17, 2020
improv: include example tests in make tests (#63)
58aa099 
* chore: add example/tests to test paths

* chore: update example tests to remove requirement to manually specify env vars

* chore: try to fix build for python3.6 (install dataclasses backport)

* chore: fix build for python 3.6

* chore: dont run example test on python3.6 since it requires asyncio
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@heitorlessa heitorlessa heitorlessa approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@to-mc @codecov-commenter @heitorlessa