Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 41 additions & 3 deletions dive_sailthru_client/client.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
from sailthru.sailthru_client import SailthruClient
from sailthru import sailthru_client
from errors import SailthruApiError
# We need the SailthruClientError to be able to handle retries in api_get
from sailthru.sailthru_error import SailthruClientError

# for patched_sailthru_http_request
from sailthru.sailthru_response import SailthruResponse
from sailthru.sailthru_http import flatten_nested_hash
import requests
import platform
# other libraries
import datetime
import time
import re
Expand All @@ -24,7 +29,40 @@ class DiveEmailTypes:
Spotlight = "spotlight"


class DiveSailthruClient(SailthruClient):
# There is some skullduggery below in order to override the hardcoded 10 second timeout on HTTP requests

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 for SAT word

# per TECH-3849. First we copy/paste the sailthru_http_request() function that originally exists here:
# https://github.com/sailthru/sailthru-python-client/blob/521fdaa30890a29da8fbb02726e7d22ed174b878/sailthru/sailthru_http.py#L30

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this referenced version allowed for a headers parameter as well though the below copy does not include it, did you take it out here on purpose?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Honestly just wrote my version before looking at theirs. I guess it's better to expose the things that anyone might want to change as parameters, but this is an internal function anyway so it's not like the end user could use headers -- it'd be for use in subclasses of the API Client

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was worried that some higher yet still internal level method in sailthru client accepts headers and depends on being able to pass them down to this method eventually (in other words, worried about getting getting a TypeError a la "takes exactly X arguments, X+1 given" if we called something that tried to send headers all the way down). I did a browse and it doesn't seem so; only for a few high level API cases for the end user to supply (i.e. https://github.com/sailthru/sailthru-python-client/blob/17e201ccfbae747ce01b71a7fd1f8470cd51621c/sailthru/sailthru_client.py#L516) which we don't even use these higher level methods in our client/possibly anywhere ever.

# Then we modify it to have a default timeout of 60 seconds and additionally to accept a timeout parameter
def timeout_patched_sailthru_http_request(url, data, method, file_data=None, timeout=60):
"""
Perform an HTTP GET / POST / DELETE request

This is a override of upstream sailthru_http_request with `timeout` added as a parameter and
with the default set to 60 instead of 10.
"""
data = flatten_nested_hash(data)
method = method.upper()
params, data = (None, data) if method == 'POST' else (data, None)

try:
headers = {'User-Agent': 'Sailthru API Python Client %s; Python Version: %s' % ('2.3.3-patched', platform.python_version())}
response = requests.request(method, url, params=params, data=data, files=file_data, headers=headers, timeout=timeout)
return SailthruResponse(response)
except requests.HTTPError as e:
raise SailthruClientError(str(e))
except requests.RequestException as e:
raise SailthruClientError(str(e))


# Now we need to patch the altered sailthru_http_request into a place where even functions defined in the upstream
# SailthruClient class that call it will call our new altered version. In the upstream class, the sailthru_http_request()
# function is `import`ed into the sailthru_client module (not the class), so we need to import the sailthru_client module
# and then redefine the sailthru_http_request that it had imported to instead point to our version. Then later we
# have to make sure we are subclassing SailthruClient by refering to it specifically as sailthru_client.SailthruClient
sailthru_client.sailthru_http_request = timeout_patched_sailthru_http_request


class DiveSailthruClient(sailthru_client.SailthruClient): # must import from sailthru_client.SailthruClient for patched HTTP timeout
"""
Our Sailthru client implementation that adds our own concepts.

Expand Down
5 changes: 5 additions & 0 deletions dive_sailthru_client/tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import datetime
import tempfile
import StringIO
import time


@attr('external')
Expand Down Expand Up @@ -37,6 +38,10 @@ def test_get_set_var(self):
value = self._get_user_var(self.test_email, self.test_var_key)
self.assertNotEqual(value, new_value)
self._set_user_var(self.test_email, self.test_var_key, new_value)
# We take a brief pause here to let Sailthru catch up. Sailthru API calls
# are only *eventually* consistent so sometimes you write a value and then
# read it back and still get the old value.
time.sleep(1)
value = self._get_user_var(self.test_email, self.test_var_key)
self.assertEqual(value, new_value)

Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

setup(
name="dive_sailthru_client",
version="0.0.15",
version="0.0.16",
description="Industry Dive abstraction of the Sailthru API client",
author='Industry Dive',
author_email='tech.team@industrydive.com',
Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[flake8]
max-line-length = 128
max-line-length = 140
max-complexity = 12
statistics = True