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

More extensibility and Tornado integration #94

Merged
merged 13 commits into from
Mar 30, 2016

Conversation

friedcell
Copy link

Updated the code to make it more extensible (easier subclassing, allows async transports) and then added support for tornado and its transport via AsyncHTTPClient

@richleland
Copy link
Contributor

Thanks for contributing @friedcell - the build is failing - can you take a look at the failures and address? https://travis-ci.org/SparkPost/python-sparkpost/builds/118036542

@@ -196,6 +196,11 @@ def send(self, **kwargs):
results = self.request('POST', self.uri, data=json.dumps(payload))
return results

def _fetch_get(self, transmission_id):
Copy link
Contributor

Choose a reason for hiding this comment

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

Any particular reason this had to be moved out into a separate function?

Copy link
Author

Choose a reason for hiding this comment

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

To support async transports fetch methods should only return whatever they fetched - in async it will be a future, not a real value that can be manipulated. It enables code in sparkpost/tornado/transmissions.py

@richleland
Copy link
Contributor

Hi @friedcell this PR looks pretty solid. I'm not very familiar with Tornado, so adding a few examples would be awesome if you can squeeze that in. It would also be great to get a section covering Tornado usage in the Sphinx docs.

@richleland richleland added this to the 1.1.0 milestone Mar 29, 2016
@richleland richleland merged commit 411c565 into SparkPost:master Mar 30, 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

2 participants