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

Various fixes and improvements #5

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Various fixes and improvements #5

wants to merge 10 commits into from

Conversation

gpoulter
Copy link

@gpoulter gpoulter commented Oct 3, 2011

  • Replace setup.py dependencies with pip requirements.txt
    • argparse is built-in on py2.7, setup.py tries to install over it.
    • boto package also found in ubuntu (on ec2 machines), setup.py tries to install over it too.
  • Fix error-handling in cmd_send by catching BotoServer error instead of checking message.
  • Improve logging in cmd_send by printing key=value pairs containing information
    about SES parameters, error code, request ID and message ID. Emitted log message of "timestamp key=value ..." format can be parsed natively by Splunk.
  • Fix list_verified to sort addresses
  • Change output of show_quota to match ses-get-stats.pl -q
  • Change output of show_stats to match output of ses-get-stats.pl -s
  • Remove out() function as it is no longer used
  • Clean up argparse usage, handle main

setup.py dependency handling is broken on Ubuntu
- argparse is built-in on py2.7, setup.py tries to install over it.
- boto also available via apt, setup.py tries to install over it too.

So created requirements.txt with boto>=2.0 instead.
Fix error-handling in cmd_send by catching BotoServer error instead of checking message.

Improved logging in cmd_send by printing key=value pairs containing information
about SES parameters, error code, request ID and message ID.

Emitted log message of "timestamp key=value ..." format can be parsed natively by Splunk.
@paltman
Copy link
Collaborator

paltman commented Oct 4, 2011

I like everything here except for the change in the setup.py, can you explain why this change is needed? The package requires boto 2.0 and therefore belongs in install-requires of setup.py.

@gpoulter
Copy link
Author

gpoulter commented Oct 5, 2011

Ok, we can leave out setup.py revision then, I'll try figure out why it's not detecting the existing deps. I don't see a button to exclude it though, either I'll have to rebase again to leave it out and redo the pull request, or (simpler) add a reverting commit.

@gpoulter
Copy link
Author

Now it will only require argparse if the Python version is known to lack a built-in argparse, is that ok?

@arcturusannamalai
Copy link

I found the new boto API has changed, and response struct has fields different than the code; current response has a format like,

{u'SendEmailResponse': {u'ResponseMetadata': {u'RequestId': u'0xLONGHEX'},
                        u'SendEmailResult': {u'MessageId': u'0xLONGHEX'}}}

due to this the postman send command will not respond correctly.

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.

3 participants