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-640] Add support for Softlayer DNS driver #413

Closed

Conversation

@vanclevstik
Copy link
Contributor

vanclevstik commented Dec 9, 2014

  • Refactored SoftLayer connection, moved it to common, to be used by compute and dns driver
  • Added support for DNS driver methods
  • Unit tests for all methods

Driver has been tested a bit manually too and it seems to work. Looking for review/any comments

One thing missing is support for "update_zone" method as SoftLayer doesn't support so I don't really know how to tackle this problem?

@vanclevstik vanclevstik force-pushed the niteoweb:create_softlayer_dns_driver branch from 2007cc9 to 81e844d Dec 9, 2014
@Kami
Copy link
Member

Kami commented Dec 9, 2014

Great, thanks. I will review it ASAP.

As far as update_zone goes - you can just leave it unimplemented. Most providers who support zone editing only allow you to change attributes such as "ttl" and "email" (email address of the zone owner).

@vanclevstik vanclevstik force-pushed the niteoweb:create_softlayer_dns_driver branch from 81e844d to 9be2d8b Dec 9, 2014
@vanclevstik
Copy link
Contributor Author

vanclevstik commented Dec 10, 2014

Had some Travis fails on py3 builds, should be fine now.

RecordType.TXT: 'txt',
}

def _to_zone(self, item):

This comment has been minimized.

Copy link
@Kami

Kami Dec 11, 2014

Member

Minor style and consistency things - can you please re-order the methods and put "private" methods at the end (https://libcloud.readthedocs.org/en/latest/development.html#function-and-method-ordering)?

This comment has been minimized.

Copy link
@vanclevstik

vanclevstik Dec 12, 2014

Author Contributor

OK will fix.


def delete_record(self, record):
try:
return self.connection.request(

This comment has been minimized.

Copy link
@Kami

Kami Dec 11, 2014

Member

The method should return True on success.

This comment has been minimized.

Copy link
@vanclevstik

vanclevstik Dec 12, 2014

Author Contributor

Response class parses the response and returns True on success, as seen in tests https://github.com/apache/libcloud/pull/413/files#diff-dfb598e3fd0504cdd5bdd96d48301ea2R144

# limitations under the License.

import sys
import unittest

This comment has been minimized.

Copy link
@Kami

Kami Dec 11, 2014

Member

Please use libcloud.test.unittest, just in case if we ever want to use functionality from unittest2 / unittest module in Python >= 2.7.

This comment has been minimized.

Copy link
@vanclevstik

vanclevstik Dec 12, 2014

Author Contributor

Aha agreed, will change it.


SoftLayerDNSMockHttp.type = 'ZONE_DOES_NOT_EXIST'

try:

This comment has been minimized.

Copy link
@Kami

Kami Dec 11, 2014

Member

When you switch to libcloud.test.unittest you can use self.assertRaises here.

This comment has been minimized.

Copy link
@vanclevstik

vanclevstik Dec 12, 2014

Author Contributor

Yep, will do it for all exception tests.

def delete_zone(self, zone):
self.connection.set_context({'resource': 'zone', 'id': zone.id})
try:
return self.connection.request(

This comment has been minimized.

Copy link
@Kami

Kami Dec 11, 2014

Member

Same here, should return True on success.

This comment has been minimized.

@vanclevstik vanclevstik force-pushed the niteoweb:create_softlayer_dns_driver branch from 9be2d8b to 0e76368 Dec 15, 2014
@vanclevstik
Copy link
Contributor Author

vanclevstik commented Dec 15, 2014

@Kami Fixed, ready for review :)

@asfgit asfgit closed this in dd7ba77 Dec 16, 2014
@Kami
Copy link
Member

Kami commented Dec 16, 2014

I've made some more missed changes (updated delete_* methods to return a boolean on success) and merged patch into trunk.

Thanks!

@dz0ny dz0ny deleted the niteoweb:create_softlayer_dns_driver branch Aug 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.