SES support

[hmarr]

Hey,

I've covered all of the documented API methods for SES, so it should be ready for merging. Let me know if there's anything un-botoish that needs changing!

Thanks,
Harry

[hmarr]

Thinking about this further, it could be more useful to process the response more. At the minute, getting results out of responses takes a fair bit of digging into dictionaries with unicode keys, which can be fairly unintuitive. However, I noticed that other service wrappers in boto (such as route53) seem to just return the raw response dict. Do you have a preference?

[garnaat]

For most of the older modules, you will find that there is quite a bit of custom code that has been created to handle the responses from AWS. Basically, a hand-coded Python class for each response type where all of the elements of the response are parsed into attributes of the object.

This approach gives you nice Python objects but is also kind of tedious to code. Starting with the IAM module, I experimented with parsing the XML responses directly into native Python objects like dicts and lists using the jsonresponse class that you have used. This makes it quite a bit easier to get a new module implemented but, as you have noticed, sometimes requires some serious spelunking into the data structures to find the info you want. To make that a little easier to deal with, I added some getattr magic to the Element class that let's you access deeply nested fields as attributes of the top-level object. This is described a bit in this blog post about IAM:

http://www.elastician.com/2010/09/using-identity-access-management-iam.html

So, I guess my current preference is to parse to native Python data structures. If that seems awkward, I'd rather try to find ways to make it easier than to revert back to hand-coded classes. Of course, if you have any other ideas, I'm always open to suggestions.

BTW, there is another pull request pending for an SES implementation. I'm looking through both to try to merge to a single code base going forward.

[gtaylor]

I commented to this effect on the other pull request, but IMHO, putting the entire contents of one of these APIs in __init__.py is really bad organization. SDB and a few of the other older modules would have all of what is currently in there in a connection.py module, so the contents are at least somewhat described. If The code was all tossed into __init__.py for the sake of making import statements smaller, that can still be done by importing the desired classes in __init__.py.

Like the other SES pull request, this looks to lack unit tests. It'd be really nice to have some that at least ran through the code paths, even if we can't check the email responses. We can find a lot of lower-level errors just by running things through.

[hmarr]

@garnaat: Ah that makes things a lot easier - didn't see that before. I'm still not 100% sure about including all the parts of the response - most methods return something like {MethodNameResponse: {MethodNameResult {...}}} - I'm not really sure from an API design point of view that it makes sense to give these structures to the user. Would much be lost by just stepping down the dictionary a level or two (i.e. return response['%sResponse' % action_name]['%sResult' % action_name])? This doesn't apply to all actions, but for the ones returning meaningful output it could be a bit more intuitive.

Also, hadn't seen the other pull request - actually looks like there are two others now! Seems they were both written after I'd pushed the first version of my implementation.

@gtaylor: Thanks for your feedback. I agree on the __init__.py point - initially I'd looked at the SNS package that follows this approach, and tried to remain consistent. But having checked out some of the other packages it doesn't look like this is in the style of the project (or even well organised code). I'll update accordingly.

Re: tests - I've actually written a couple to run through the code, but they require valid email addressees to pass, linked to an AWS account so I didn't include them. Do you reckon just mocking out the responses would suffice?

[gtaylor]

That's a good question, regarding the mock stuff. I think that approach is used for some of the other modules, but garnaat would definitely be able to say with better authority if that's what he wants going on into the future. It seems like that might be the way to go, though, as long as we keep our test mock-XML up to date with SES as it leaves beta.

[garnaat]

Re: tests - We need much better and more comprehensive tests in boto. Ultimately, I think we have to test against live services but since that's complicated, time-consuming and potentially costly, we do need alternatives. I'm not a huge fan of mocking services but there is no doubt that it would catch certain types of errors and having a way to anyone to run boto unit tests without worrying about AWS credentials would be a big plus. I'm hoping that the roboto package I'm working on now will be a step in the right direction wrt mocking services responses but I still have a lot of work to do on that before it becomes generally useful.

Re: init.py: For some reason I find it difficult to generate much angst about this. Older modules generally have empty init.py files because they had lots of support files and adding an additional one for connection.py made sense. Some later modules essentially defined no additional classes so it seemed to make more sense to just include the Connection class in init.py. I guess the best argument for changing those is consistency.

[garnaat]

I'd like to get a version of SES available in boto very soon. I haven't had the time to try to merge the three submissions. Have you had a chance to look at the others?

[gtaylor]

@garnaat: Not sure if that question was directed at hmarr or me, but I did look around at the other pull requests. It all looks similar, but I think hmarr has the most documentation and the best public API so far.

I did some further commenting on the latest commits with some documentation related stuff, and some privatization of what appear to be private methods. Given the resolution of these, this pull request has my vote, at least.

hmarr has already got a django-ses module in the works too, which I'd be interested in for my own selfish purposes :)

[hmarr]

@gtaylor: Thanks a lot for your comments, I'll update my fork to relect the changes.

[hmarr]

Re: return types - garnaat mentioned that native Python types is the way to go rather than custom objects. That's how the code currently works, but just returning a dictified version of the API response results in some pretty hairy structures:

response = conn.list_verified_email_addresses()[u'ListVerifiedEmailAddressesResponse']
result = response[u'ListVerifiedEmailAddressesResult']
for address in result[u'VerifiedEmailAddresses']:
    print address

It would be fairly trivial to only return the 'relevant' data (in this example, just return the list of addresses). However, some of the other service wrappers appear to just return the raw dict so I wouldn't want to be inconsistent. Which approach to you reckon is better?

[garnaat]

There was a thread on boto-users about this, too. Here's what I think should happen but I'm open to suggestions.

I think there should first be a low-level implementation where all of the methods are implemented on the Connection object and each of the methods returns the raw Python data structures created by jsonresponse.py. This would allow us to get a functioning module up relatively quickly.

The next step or next layer on top of this would be a set of Python classes that abstract the low-level data structures and add lots of convenience methods to the higher-level classes. This would involve (at least for the moment) writing custom Python classes but the result would look like earlier boto modules.

Finally, I would like to see some sort of switch in the Connection object or something that would allow you to toggle between the two different style results. Or maybe it's done on a per-request basis. Haven't really thought that one through yet.

This approach seems to provide the best of both worlds. Thoughts?

[hmarr]

Having a comprehensive, consistent set of low-level API wrappers makes sense to me, I agree that it would make a good base to provide abstractions on top of.

Were you thinking that the layers above would be part of the core boto library or provided by external wrapper libraries?

I'm not too sure about being able to switch between different result styles, having to look at object state before calling methods could be quite confusing. Perhaps connection subclasses that provide higher-level APIs could make more sense?

[garnaat]

I was thinking of the higher-level layers as being part of boto but I guess they wouldn't have to be. For example, the current route53 module returns dict objects as does the SES. But another boto user has written additional classes to create the higher-level classes and those can reside right in the same module.

Different connection classes sounds like a reasonable approach for exposing the different layers.

[hmarr]

Great, I'll go ahead with documenting the return values of the low-level api. I'm thinking examples might be the easiest way of describing the return dicts - code samples would probably be more helpful than sentences describing the dicts' structures.

[hmarr]

I've just pushed a slightly refactored version - the return types are still slightly cryptic due to the structures returned by the SES api. I could include examples in the main section of the docstring, e.g.:

{
    u'ListVerifiedEmailAddressesResponse': {
        u'ListVerifiedEmailAddressesResult': {
            u'VerifiedEmailAddresses': [u'person1@example.com', u'person2@example.com']
        },
        u'ResponseMetadata': {
            u'RequestId': u'asd9f9sa-asdf9as0f-asfdasf0da-asdfas'
        }
    }
}

But maybe that's not necessary - would make the docs quite a bit longer, and the dicts mostly mirror the SES api, which is documented.

[gtaylor]

IMHO, as long as the users can print/log/emit the dicts (or a class with informative repr(obj) output), it's good enough for now. Especially given that this is a beta AWS service that may change, it'd stink to have out of date data on it.

This is looking like a pretty strong pull request now, it definitely has my vote. Excellent job, hmarr.

[hmarr]

Yea I think that'll be fine. Thanks a lot for all your help!

[garnaat]

This pull request has been merged.