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

Change AWS `generate_credentials` request method to GET #475

Merged
merged 2 commits into from Jun 18, 2019

Conversation

Projects
None yet
3 participants
@donjar
Copy link
Contributor

commented Jun 14, 2019

No description provided.

@donjar donjar force-pushed the donjar:patch-1 branch from eea2f61 to 0a0aaf4 Jun 17, 2019

@codecov-io

This comment has been minimized.

Copy link

commented Jun 17, 2019

Codecov Report

Merging #475 into develop will decrease coverage by 0.37%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #475      +/-   ##
===========================================
- Coverage    88.43%   88.05%   -0.38%     
===========================================
  Files           50       51       +1     
  Lines         2664     2680      +16     
===========================================
+ Hits          2356     2360       +4     
- Misses         308      320      +12
Impacted Files Coverage Δ
hvac/api/secrets_engines/aws.py 100% <100%> (ø) ⬆️
hvac/api/system_backend/capabilities.py 20% <0%> (ø)
hvac/api/system_backend/__init__.py 95.83% <0%> (+0.18%) ⬆️

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 897a41a...fe3d3df. Read the comment docs.

@@ -326,7 +326,7 @@ def generate_credentials(self, name, role_arn=None, ttl="3600s", endpoint='creds
name=name,
)

response = self._adapter.post(
response = self._adapter.get(
url=api_path,
json=params,

This comment has been minimized.

Copy link
@jeffwecan

jeffwecan Jun 17, 2019

Collaborator

Does the json keyword are work with requests.get()? If not, this should be updated to params=params, no?

This comment has been minimized.

Copy link
@donjar

donjar Jun 18, 2019

Author Contributor

It does work when I tested it, but I think params makes more sense. Let me update it.

@jeffwecan
Copy link
Collaborator

left a comment

Looks good, thanks @donjar!

@jeffwecan jeffwecan merged commit c013371 into hvac:develop Jun 18, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jeffwecan jeffwecan added aws bug labels Jul 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.