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

Translate service name to lower case #444

Merged
merged 1 commit into from
Jun 28, 2021
Merged

Translate service name to lower case #444

merged 1 commit into from
Jun 28, 2021

Conversation

blakemorgan
Copy link
Contributor

@blakemorgan blakemorgan commented Jun 18, 2021

Issue #, if available: #443

Description of changes: When using the AWS SDK v3, service names are uppercase. The call_capturer and whitelisting works under the assumption that all services are lower case and one word (this assumption was derived from the default whitelist). Switching the service name to lowercase stopped the debug logs from saying that calls weren't whitelisted.

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

fixes #443

Copy link
Contributor

@bhautikpip bhautikpip left a comment

Choose a reason for hiding this comment

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

Looks good :)

@codecov-commenter
Copy link

Codecov Report

Merging #444 (5c9ad82) into master (df97b82) will decrease coverage by 0.11%.
The diff coverage is n/a.

❗ Current head 5c9ad82 differs from pull request most recent head 86c7872. Consider uploading reports for the commit 86c7872 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #444      +/-   ##
==========================================
- Coverage   82.59%   82.48%   -0.12%     
==========================================
  Files          36       36              
  Lines        1741     1741              
==========================================
- Hits         1438     1436       -2     
- Misses        303      305       +2     
Impacted Files Coverage Δ
lib/patchers/call_capturer.js 89.23% <0.00%> (-3.08%) ⬇️

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 df97b82...86c7872. Read the comment docs.

Copy link
Contributor

@willarmiros willarmiros left a comment

Choose a reason for hiding this comment

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

LGTM - AWS SDK v2 service names are lowercase by default so that was the difference here.

@blakemorgan
Copy link
Contributor Author

Thanks, all. I'm new to contributing here. What is the timeline usually like for getting these merged and released?

@srprash srprash merged commit 73e6ba8 into aws:master Jun 28, 2021
@srprash
Copy link
Contributor

srprash commented Jun 28, 2021

Hi @blakemorgan
The PR has been merged. We'll try to get the release out in the next couple of weeks. Stay tuned! :)

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.

Default whitelist not working with AWS SDK v3 clients
5 participants