Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Adding missing health_check value for weighted record sets. #2195

Merged
merged 2 commits into from Jul 11, 2014
Merged

Adding missing health_check value for weighted record sets. #2195

merged 2 commits into from Jul 11, 2014

Conversation

jzbruno
Copy link
Contributor

@jzbruno jzbruno commented Apr 1, 2014

Fix for issue #2194.

@jzbruno
Copy link
Contributor Author

jzbruno commented Apr 1, 2014

I didn't see any test cases for this area of the code. Any guidance on how I should proceed?

@danielgtaylor
Copy link
Member

Thanks for catching this oversight. The PR looks good, but does need tests. You can model your test on one like the following:

class TestCreateZoneRoute53(AWSMockServiceTestCase):
    connection_class = Route53Connection

    def default_body(self):
        return """
<CreateHostedZoneResponse xmlns="https://route53.amazonaws.com/doc/2012-02-29/">
    <HostedZone>
        <Id>/hostedzone/Z11111</Id>
        <Name>example.com.</Name>
        <CallerReference>aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee</CallerReference>
        <Config>
            <Comment></Comment>
        </Config>
        <ResourceRecordSetCount>2</ResourceRecordSetCount>
    </HostedZone>
    <ChangeInfo>
        <Id>/change/C1111111111111</Id>
        <Status>PENDING</Status>
        <SubmittedAt>2014-02-02T10:19:29.928Z</SubmittedAt>
    </ChangeInfo>
    <DelegationSet>
        <NameServers>
            <NameServer>ns-100.awsdns-01.com</NameServer>
            <NameServer>ns-1000.awsdns-01.co.uk</NameServer>
            <NameServer>ns-1000.awsdns-01.org</NameServer>
            <NameServer>ns-900.awsdns-01.net</NameServer>
        </NameServers>
    </DelegationSet>
</CreateHostedZoneResponse>
        """

    def test_create_zone(self):
        self.set_http_response(status_code=201)
        response = self.service_connection.create_zone("example.com.")

        self.assertTrue(isinstance(response, Zone))
        self.assertEqual(response.id, "Z11111")
        self.assertEqual(response.name, "example.com.")

https://github.com/boto/boto/blob/develop/tests/unit/route53/test_connection.py#L91-L131

In it, you create a mocked response, do the call and then check the built response object to make sure health_check is populated correctly. Give it a try and if you need more help let me know.

@dwayn
Copy link

dwayn commented Apr 16, 2014

To others that are using boto as an external thirdparty only (ie. you don't want to make people that use your code have to have a custom patched version of boto), you can do something like this to monkeypatch the endElement function on the boto class (version 2.27.0 only) as an interim solution until this PR is merged (note: you have to implement the monkey patch after you have called boto.connect_route53() due to the dynamic loading of modules)

        def boto_route53_record_Record_endElement(self, name, value, connection):
            if name == 'Name':
                self.name = value
            elif name == 'Type':
                self.type = value
            elif name == 'TTL':
                self.ttl = value
            elif name == 'Value':
                self.resource_records.append(value)
            elif name == 'HostedZoneId':
                self.alias_hosted_zone_id = value
            elif name == 'DNSName':
                self.alias_dns_name = value
            elif name == 'SetIdentifier':
                self.identifier = value
            elif name == 'EvaluateTargetHealth':
                self.alias_evaluate_target_health = value
            elif name == 'Weight':
                self.weight = value
            elif name == 'Region':
                self.region = value
            # the following 2 lines are all that this differs from boto 2.27.0
            elif name == 'HealthCheckId':
                self.health_check = value

        boto.route53.record.Record.endElement = boto_route53_record_Record_endElement

@jzbruno
Copy link
Contributor Author

jzbruno commented Apr 16, 2014

Sorry, I have been busy and forgot to finish this up. I will try to get to it tomorrow afternoon.

@jzbruno
Copy link
Contributor Author

jzbruno commented Apr 25, 2014

OK. Added checks to the existing get all rrsets test.

@jzbruno
Copy link
Contributor Author

jzbruno commented Jun 12, 2014

Is there anything holding this up from being merged? Just checking in, we are currently running a custom fork of boto and would like to get back to the official repo in the next release if possible.

@danielgtaylor danielgtaylor merged commit bf24406 into boto:develop Jul 11, 2014
danielgtaylor added a commit that referenced this pull request Jul 11, 2014
Conflicts:
	boto/route53/record.py
@danielgtaylor
Copy link
Member

Sorry, this looks good now, thanks!

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

Successfully merging this pull request may close these issues.

None yet

3 participants