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

[LIBCLOUD-813] create record and update record was not working #775

Closed
wants to merge 1 commit into from
Closed

[LIBCLOUD-813] create record and update record was not working #775

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 25, 2016

Create record and update record issues were fixed

Description

Link to the bug report: https://issues.apache.org/jira/browse/LIBCLOUD-813

Following changes were required to get those working:

  • Priority entry exists only at MX and SRV records
  • Content type of submitted data had to be application/json
  • Data parameter of connection request has to be string
    Only MX and SRV records have pririoty. Cotent-type for data has to be json. data param of connection request must be string, changed that.

Status

  • done, ready for review

    Checklist (tick everything that applies)

  • [X ] Code linting (required, can be done after the PR checks)
  • [NA ] Documentation
  • Tests - executed
  • [ NA] ICLA (required for bigger changes)

…e pririoty. Content-type for data has to be json. data param of connection request must be string, changed that.
@Kami
Copy link
Member

Kami commented Apr 25, 2016

Thanks, the changes look good to me.

If you get a chance it would also be good to add some tests which verify that the payload is indeed correct (you would need to assert this into the MockHttp class methods).

@ghost
Copy link
Author

ghost commented Apr 25, 2016

Actually all tests should be re-done because they didn't notice any critical bugs. Unfortunately I don't have time for that at the moment. LIBCLOUD-811 and -813 delayed me too much with my work.

@tonybaloney
Copy link
Contributor

LGTM

@Kami
Copy link
Member

Kami commented Apr 26, 2016

@teemuvesala Yeah, I agree.

I recently also enabled pylint so this should also help with catching at least part of the similar issues in the future.

@asfgit asfgit closed this in a268256 Apr 28, 2016
tonybaloney pushed a commit to NTTLimitedRD/libcloud that referenced this pull request May 25, 2016
…tent-type for data has to be json. data param of connection request must be string, changed that. Closes apache#775   Closes apache#736   Closes apache#736
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.

2 participants