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

auroradns: Properly parse TTL as extra data #675

Closed
wants to merge 1 commit into from

Conversation

wido
Copy link
Contributor

@wido wido commented Jan 7, 2016

Small typo in the list caused this not to be parsed properly.

@jimbobhickville
Copy link
Contributor

lgtm but I don't have a commit bit, so someone else will need to check it.

@wido
Copy link
Contributor Author

wido commented Jan 8, 2016

Probably @Kami or @Runseb ?

@Kami
Copy link
Member

Kami commented Jan 8, 2016

Good catch :)

Test would also be nice.

@wido
Copy link
Contributor Author

wido commented Jan 8, 2016

I agree @Kami, a test would indeed be a good thing :)

@wido
Copy link
Contributor Author

wido commented Jan 8, 2016

I just added a Unit Test to verify this case. I'll add more later in a different PR

AuroraDNSDriverMockHttp.type = None
self.driver = AuroraDNSDriver(*DNS_PARAMS_AURORADNS)

def test_merge_extra_data(self):
Copy link
Member

Choose a reason for hiding this comment

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

I was more thinking about adding a test case which uses response fixture, but that works for now.

@Kami
Copy link
Member

Kami commented Jan 8, 2016

Tests are failing (https://travis-ci.org/apache/libcloud/jobs/101036033), need to add aurora params to libcloud/test/secrets.py-test :)

@wido wido force-pushed the auroradns-ttl-fix branch 2 times, most recently from 1a3ad13 to 9a747d3 Compare January 8, 2016 12:18
Small typo in the list caused this not to be parsed properly
@wido
Copy link
Contributor Author

wido commented Jan 8, 2016

Fixed! Tests are no longer failing.

@Kami
Copy link
Member

Kami commented Jan 8, 2016

Merged, thanks!

@asfgit asfgit closed this in a57c808 Jan 8, 2016
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.

None yet

3 participants