Skip to content

Conversation

cortez7
Copy link
Contributor

@cortez7 cortez7 commented Jul 29, 2020

The removed account ID is the customer account ID instead of the provider account ID. This means that for public types, a new metric namespace would be created each time we need to emit metrics. The main issue with this is that the internal and logging failure alarms will not work properly and service teams may not be notified of issues.

Go plugin PR: aws-cloudformation/cloudformation-cli-go-plugin#151
Python plugin PR: aws-cloudformation/cloudformation-cli-python-plugin#108

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link

@wbingli wbingli left a comment

Choose a reason for hiding this comment

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

Could you also apply the same change to Go and Python plugins?

BTW, build fails. you can run pre-commit run --all-files to check.

@wbingli wbingli self-requested a review July 29, 2020 20:53
@cortez7 cortez7 removed the request for review from AugustXiaodm July 29, 2020 22:30
@cortez7
Copy link
Contributor Author

cortez7 commented Jul 29, 2020

When I ran pre-commit run --all-files everything passed, although beforehand I hand to run pip install pylint and pip install pytest-cov. We decided to fix the build issue in a different PR. We may need to lock the versions.

@cortez7
Copy link
Contributor Author

cortez7 commented Jul 30, 2020

Build failures are now fixed with new pytest release.

Copy link

@wbingli wbingli left a comment

Choose a reason for hiding this comment

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

Thanks

@wbingli wbingli merged commit e37eccc into aws-cloudformation:master Jul 30, 2020
@cortez7 cortez7 deleted the metric-namespace-fix branch July 30, 2020 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants