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

Add support for Google DNS API. #269

Closed
wants to merge 1 commit into from
Closed

Add support for Google DNS API. #269

wants to merge 1 commit into from

Conversation

@fcuny
Copy link
Contributor

@fcuny fcuny commented Mar 30, 2014

Google recently added a new API for DNS management. This new driver
implement simple operations to create and view zones and records.

I still need to add a few more tests and probably add a bunch of things into the 'extra' for records and zones.

I'm also not familiar with other DNS drivers, so a few things might be wrong or should be done differently.


def __init__(self, user_id, key, secure, auth_type=None,
credential_file=None, project=None, **kwargs):
self.scope = ['https://www.googleapis.com/auth/ndev.clouddns.readwrite']

This comment has been minimized.

@Kami

Kami Mar 31, 2014
Member

Can you please explicitly declare all the supports arguments instead of (ab)using kwargs?

:rtype: :class:`Record`
"""
for record in self.iterate_records(self.get_zone(zone_id)):
r_id = "%s-%s" % (record.name, record.type)

This comment has been minimized.

@Kami

Kami Mar 31, 2014
Member

For consistency, please use single quotes around strings.

:rtype: :class:`Record`
"""
for record in self.iterate_records(self.get_zone(zone_id)):

This comment has been minimized.

@Kami

Kami Mar 31, 2014
Member

I guess there is no more efficient way to do that?

This comment has been minimized.

@fcuny

fcuny Apr 5, 2014
Author Contributor

Actually there is. I've pushed an updated version with a better solution.

@Kami
Copy link
Member

@Kami Kami commented Apr 7, 2014

@franckcuny Thanks for addressing the comments.

I'll do another review shortly and let you know if I encounter any more issues.

'type': record_type,
}

request = '/managedZones/%s/rrsets' % zone_id

This comment has been minimized.

@Kami

Kami Apr 7, 2014
Member

Minor thing - to prevent accidental bugs, please always use parenthesis around format string arguments.

:rtype: :class:`Record`
"""
(record_name, record_type) = record_id.split("-")

This comment has been minimized.

@Kami

Kami Apr 7, 2014
Member

For consistency, please use single quotes around strings (except docstrings).

def _get_more(self, rtype, **kwargs):
last_key = None
exhausted = False
while exhausted is False:

This comment has been minimized.

@Kami

Kami Apr 7, 2014
Member

while not exhausted is a bit more ideomatic

@Kami
Copy link
Member

@Kami Kami commented Apr 7, 2014

Besides some minor style and consistency issues, it looks good to me.

@Kami
Copy link
Member

@Kami Kami commented Apr 7, 2014

Please also sync this branch with trunk so Travis build won't fail anymore.

Google recently added a new API for DNS management. This new driver
implement simple operations to create and view zones and records.
@erjohnso
Copy link
Member

@erjohnso erjohnso commented Apr 28, 2014

@Kami, @franckcuny - is this good to go? I am intending to add an Ansible module and would love to take advantage of this work. :)

@Kami
Copy link
Member

@Kami Kami commented Apr 28, 2014

@erjohnso Sorry for the delay. I've just merged those changes into trunk.

@erjohnso
Copy link
Member

@erjohnso erjohnso commented Apr 28, 2014

@Kami thank you sir!

@asfgit asfgit closed this in 8654313 Apr 28, 2014
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

3 participants