ses: SetIdentityFeedbackForwardingEnabled and SetIdentityNotificationTopic #2130

Merged
merged 2 commits into from Mar 10, 2014

Conversation

Projects
None yet
3 participants
Contributor

nhumphreys commented Feb 27, 2014

Issue #2128

Contributor

toastdriven commented Feb 27, 2014

Before this can be merged, it will need some unit tests. Something like https://github.com/boto/boto/blob/develop/tests/unit/ses/test_identity.py#L30-L79 (with appropriate bodies/assertions for the additions) would be perfect. Otherwise, it will need to wait until a core dev can add them.

Contributor

nhumphreys commented Feb 27, 2014

Added some unit tests, hopefully they are what is required.
These two set methods return boring results as per the AWS API

Owner

danielgtaylor commented Feb 27, 2014

This looks great and the tests are passing, however before merging this in I'd love to see you add a request assertion test similar to https://github.com/boto/boto/blob/develop/tests/unit/ec2/test_connection.py#L140

class TestPurchaseReservedInstanceOffering(TestEC2ConnectionBase):
    def default_body(self):
        return """<PurchaseReservedInstancesOffering />"""

    def test_serialized_api_args(self):
        self.set_http_response(status_code=200)
        response = self.ec2.purchase_reserved_instance_offering(
                'offering_id', 1, (100.0, 'USD'))
        self.assert_request_parameters({
            'Action': 'PurchaseReservedInstancesOffering',
            'InstanceCount': 1,
            'ReservedInstancesOfferingId': 'offering_id',
            'LimitPrice.Amount': '100.0',
            'LimitPrice.CurrencyCode': 'USD',},
             ignore_params_values=['AWSAccessKeyId', 'SignatureMethod',
                                   'SignatureVersion', 'Timestamp',
                                   'Version'])

If you can't get it working let me know and I'll take a stab at it. Thanks!

@danielgtaylor danielgtaylor self-assigned this Feb 27, 2014

Contributor

nhumphreys commented Feb 28, 2014

Hi, With my limited experience I've given adding a request assertion a go, I'm guessing I haven't properly init something further up. Or maybe I'm just doing it wrong :(

Here is the test_connection.py I made for ses

#!/usr/bin/env python

from tests.unit import unittest
from tests.unit import AWSMockServiceTestCase

from boto.jsonresponse import ListElement
from boto.ses.connection import SESConnection

class TestSESConnectionBase(AWSMockServiceTestCase):
    connection_class = SESConnection

    def setUp(self):
        super(TestSESConnectionBase, self).setUp()
        self.ses = self.service_connection

class TestSESSetIdentityNotificationTopicResponse(TestSESConnectionBase):
    def default_body(self):
        return """<SetIdentityNotificationTopic />"""

    def test_serialized_api_args(self):
        self.set_http_response(status_code=200)
        response = self.ses.set_identity_notification_topic(
                        identity='user@example.com',
                        notification_type='Bounce',
                        sns_topic='arn:aws:sns:us-east-1:123456789012:example',
                        )
        self.assert_request_parameters({
           'Action': 'SetIdentityNotificationTopic',
           'Identity': 'user@example.com',
           'NotificationType': 'Bounce',
           'SnsTopic': 'arn:aws:sns:us-east-1:123456789012:example',},
           ignore_params_values=['AWSAccessKeyId', 'SignatureMethod',
                                   'SignatureVersion', 'Timestamp',
                                   'Version'])


if __name__ == '__main__':
    unittest.main()

Fails:

 assert_request_parameters
    self.assertIn(param, request_params)
AssertionError: 'AWSAccessKeyId' not found in {}

It would be great if you could have a stab at it.

Owner

danielgtaylor commented Mar 10, 2014

@nhumphreys sorry I made a mistake in the type of service this is - you can't use assert_request_parameters like I was showing you. Very sorry for the confusion I must have caused. This looks good as-is and I'm going to merge it in. Later we can add more tests if needed, but the response handling tests look like they cover most of the new code.

danielgtaylor added a commit that referenced this pull request Mar 10, 2014

Merge pull request #2130 from nhumphreys/ses-notification-support
Add SetIdentityFeedbackForwardingEnabled and SetIdentityNotificationTopic for SES. Fixes #2130, #2128.

@danielgtaylor danielgtaylor merged commit 83002d5 into boto:develop Mar 10, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment