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

dns/digitalocean: send attributes in body for PUT and POST operations. #1505

Merged
merged 2 commits into from Nov 5, 2020

Conversation

andrewsomething
Copy link
Contributor

@andrewsomething andrewsomething commented Oct 14, 2020

dns/digitalocean: send attributes in body for PUT and POST operations.

Description

Like with the DigitalOcean compute driver, requests for the DNS driver should send data in the body for PUT and POST operations rather than in query parameters. e.g. https://developers.digitalocean.com/documentation/v2/#create-a-new-domain-record

Status

  • done, ready for review

Checklist (tick everything that applies)

  • Code linting (required, can be done after the PR checks)
  • Documentation
  • Tests
  • ICLA (required for bigger changes) (I believe I signed this some time ago, not sure how to check if that's the case)

@codecov-io
Copy link

Codecov Report

Merging #1505 into trunk will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##            trunk    #1505      +/-   ##
==========================================
- Coverage   84.77%   84.77%   -0.01%     
==========================================
  Files         388      388              
  Lines       81521    81521              
  Branches     8243     8243              
==========================================
- Hits        69111    69109       -2     
- Misses       9422     9423       +1     
- Partials     2988     2989       +1     
Impacted Files Coverage Δ
libcloud/dns/drivers/digitalocean.py 92.59% <100.00%> (ø)
libcloud/test/dns/test_base.py 97.01% <0.00%> (-2.99%) ⬇️

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 d5e957a...ee5c1f7. Read the comment docs.

Copy link

@scotchneat scotchneat left a comment

Choose a reason for hiding this comment

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

👍

@Kami
Copy link
Member

Kami commented Nov 1, 2020

Thanks for the contribution.

The changes look good to me, but it would be good to add some tests for it (should be pretty straight forward - just need to update mock http classes to assert on the request body).

@andrewsomething
Copy link
Contributor Author

Good call. Changed tests fail on trunk and pass against this branch.

@asfgit asfgit merged commit 63fba89 into apache:trunk Nov 5, 2020
@Kami
Copy link
Member

Kami commented Nov 5, 2020

Thanks for adding the tests.

I've added a small change (9b72e75) and merged changes into trunk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants