-
Notifications
You must be signed in to change notification settings - Fork 20
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
Enable dev mode #76
Enable dev mode #76
Changes from 9 commits
01d89e0
c6489fe
dd5ac0b
e227c10
16f7ace
9652fa2
85a0fb5
5db8194
982eeee
de20f5a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,11 @@ | |
import backoff | ||
import singer | ||
|
||
from datetime import timedelta | ||
from singer.utils import now | ||
from requests.exceptions import ChunkedEncodingError, Timeout, ConnectionError | ||
from tap_typeform.utils import write_config | ||
|
||
Comment on lines
2
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. from datetime import timedelta
import backoff
from requests.exceptions import ChunkedEncodingError, Timeout, ConnectionError
import singer
from singer.utils import now
from tap_typeform.utils import write_config i suggest keeping standard imports first |
||
|
||
LOGGER = singer.get_logger() | ||
|
||
|
@@ -113,22 +117,69 @@ class Client(object): | |
The client class is used for making REST calls to the Github API. | ||
""" | ||
BASE_URL = 'https://api.typeform.com' | ||
OAUTH_URL = 'https://api.typeform.com/oauth/token' | ||
|
||
def __init__(self, config): | ||
self.token = 'Bearer ' + config.get('token') | ||
def __init__(self, config, config_path, dev_mode): | ||
self.metric = config.get('metric') | ||
self.session = requests.Session() | ||
self.page_size = MAX_RESPONSES_PAGE_SIZE | ||
self.form_page_size = FORMS_PAGE_SIZE | ||
self.config_path = config_path | ||
self.get_page_size(config) | ||
|
||
self.client_id = config.get('client_id') | ||
self.client_secret = config.get('client_secret') | ||
self.refresh_token = config.get('refresh_token') | ||
self.access_token = config.get('token') | ||
self.dev_mode = dev_mode | ||
self.refresh() | ||
|
||
# Set and pass request timeout to config param `request_timeout` value. | ||
config_request_timeout = config.get('request_timeout') | ||
if config_request_timeout and float(config_request_timeout): | ||
self.request_timeout = float(config_request_timeout) | ||
else: | ||
self.request_timeout = REQUEST_TIMEOUT # If value is 0,"0","" or not passed then it set default to 300 seconds. | ||
|
||
@backoff.on_exception(backoff.expo,(Timeout, ConnectionError), # Backoff for Timeout and ConnectionError. | ||
max_tries=5, factor=2, jitter=None) | ||
@backoff.on_exception(backoff.expo, (TypeformInternalError, TypeformNotAvailableError, TypeformTooManyError, ChunkedEncodingError), | ||
max_tries=3, factor=2) | ||
def refresh(self): | ||
""" | ||
Refreshes access token and refresh token | ||
""" | ||
# Existing connections won't have refresh token so use the existing access token | ||
if not self.refresh_token: | ||
return | ||
|
||
# In dev mode, don't refresh access token | ||
if self.dev_mode: | ||
if not self.access_token: | ||
raise Exception('Access token is missing') | ||
|
||
return | ||
|
||
response = self.session.post(url=self.OAUTH_URL, | ||
headers={ | ||
'Content-Type': 'application/x-www-form-urlencoded'}, | ||
data={'client_id': self.client_id, | ||
'client_secret': self.client_secret, | ||
'refresh_token': self.refresh_token, | ||
'grant_type': 'refresh_token', | ||
'scope': 'forms:read accounts:read images:read responses:read themes:read workspaces:read offline'}) | ||
|
||
if response.status_code != 200: | ||
raise_for_error(response) | ||
|
||
data = response.json() | ||
self.refresh_token = data['refresh_token'] | ||
self.access_token = data['access_token'] | ||
|
||
write_config(self.config_path, | ||
{'refresh_token': self.refresh_token, | ||
'token': self.access_token}) | ||
|
||
def get_page_size(self, config): | ||
""" | ||
This function will get page size from config, | ||
|
@@ -138,7 +189,7 @@ def get_page_size(self, config): | |
if page_size is None: | ||
return | ||
if ((type(page_size) == int or type(page_size) == float) and (page_size > 0)) or \ | ||
(type(page_size) == str and page_size.replace('.', '', 1).isdigit() and (float(page_size) > 0) ): | ||
(type(page_size) == str and page_size.replace('.', '', 1).isdigit() and (float(page_size) > 0)): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can simplify this by a small block instead of using a function to parse page_size in the init try:
page_size = int(config.get('page_size') or 0)
if page_size > 0:
self.page_size = page_size
self.form_page_size = min(self.form_page_size, self.page_size)
else:
raise ValueError("Negative or invalid ....")
except (ValueError, TypeError) as err:
LOGGER.info("Invalid Value: %s, for page_size", config.get('email_activity_date_window', 0))
raise from err There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is unintentional refactoring change without any logical change. I would not prefer to cause any regression because of new changes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. okay |
||
self.page_size = int(float(page_size)) | ||
self.form_page_size = min(self.form_page_size, self.page_size) | ||
else: | ||
|
@@ -150,19 +201,19 @@ def build_url(self, endpoint): | |
""" | ||
return f"{self.BASE_URL}/{endpoint}" | ||
|
||
@backoff.on_exception(backoff.expo,(Timeout, ConnectionError), # Backoff for Timeout and ConnectionError. | ||
max_tries=5, factor=2, jitter=None) | ||
@backoff.on_exception(backoff.expo, (Timeout, ConnectionError), # Backoff for Timeout and ConnectionError. | ||
max_tries=5, factor=2, jitter=None) | ||
@backoff.on_exception(backoff.expo, (TypeformInternalError, TypeformNotAvailableError, TypeformTooManyError, ChunkedEncodingError), | ||
max_tries=3, factor=2) | ||
max_tries=3, factor=2) | ||
def request(self, url, params={}, **kwargs): | ||
""" | ||
Call rest API and return the response in case of status code 200. | ||
""" | ||
|
||
if 'headers' not in kwargs: | ||
kwargs['headers'] = {} | ||
if self.token: | ||
kwargs['headers']['Authorization'] = self.token | ||
if self.access_token: | ||
kwargs['headers']['Authorization'] = 'Bearer ' + self.access_token | ||
|
||
LOGGER.info("URL: %s and Params: %s", url, params) | ||
response = self.session.get(url, params=params, headers=kwargs['headers'], timeout=self.request_timeout) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import json | ||
|
||
|
||
def read_config(config_path): | ||
""" | ||
Performs read on the provided filepath, | ||
returns empty dict if invalid path provided | ||
""" | ||
try: | ||
with open(config_path, "r") as tap_config: | ||
return json.load(tap_config) | ||
except FileNotFoundError as err: | ||
raise Exception("Failed to load config in dev mode") from err | ||
|
||
|
||
def write_config(config_path, data): | ||
""" | ||
Updates the provided filepath with json format of the `data` object | ||
""" | ||
config = read_config(config_path) | ||
config.update(data) | ||
with open(config_path, "w") as tap_config: | ||
json.dump(config, tap_config, indent=2) | ||
return config |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
import unittest | ||
import os | ||
import json | ||
import requests | ||
from unittest import mock | ||
|
||
from tap_typeform.client import Client | ||
|
||
|
||
http_response = {"refresh_token": "new_refresh_token", | ||
"access_token": "new_access_token"} | ||
|
||
test_config_path = "/tmp/test_config.json" | ||
|
||
|
||
|
||
def write_new_config_file(**kwargs): | ||
test_config = {} | ||
with open(test_config_path, "w") as config: | ||
for key, value in kwargs.items(): | ||
test_config[key] = value | ||
config.write(json.dumps(test_config)) | ||
|
||
|
||
class Mockresponse: | ||
""" Mock response object class.""" | ||
|
||
def __init__(self, status_code, raise_error, text=""): | ||
self.status_code = status_code | ||
self.raise_error = raise_error | ||
self.text = text | ||
|
||
def raise_for_status(self): | ||
if not self.raise_error: | ||
return self.status_code | ||
|
||
raise requests.HTTPError("Sample message") | ||
|
||
def json(self): | ||
""" Response JSON method.""" | ||
return self.text | ||
|
||
|
||
def get_mock_http_response(status_code): | ||
"""Return http mock response.""" | ||
response = requests.Response() | ||
response.status_code = status_code | ||
return response | ||
|
||
|
||
def get_response(status_code, raise_error=True, text=""): | ||
""" Returns required mock response. """ | ||
return Mockresponse(status_code, raise_error=raise_error, text=text) | ||
|
||
|
||
class TestDevMode(unittest.TestCase): | ||
def tearDown(self): | ||
if os.path.isfile(test_config_path): | ||
os.remove(test_config_path) | ||
|
||
@mock.patch("requests.Session.request") | ||
def test_dev_mode_not_enabled(self, mock_post_request): | ||
test_config = {"refresh_token": "old_refresh_token", | ||
"token": "old_access_token"} | ||
write_new_config_file(**test_config) | ||
mock_post_request.side_effect = [get_response(200, raise_error=False, text=http_response)] | ||
client = Client(config=test_config, config_path=test_config_path, dev_mode=False) | ||
self.assertEqual(client.refresh_token, "new_refresh_token") | ||
self.assertEqual(client.access_token, "new_access_token") | ||
|
||
@mock.patch("requests.Session.request") | ||
def test_dev_mode_enabled(self, mock_post_request): | ||
test_config = {"refresh_token": "old_refresh_token", | ||
"token": "old_access_token"} | ||
write_new_config_file(**test_config) | ||
mock_post_request.side_effect = [get_response(200, raise_error=False, text=http_response)] | ||
client = Client(config=test_config, config_path=test_config_path, dev_mode=True) | ||
self.assertEqual(client.refresh_token, "old_refresh_token") | ||
self.assertEqual(client.access_token, "old_access_token") | ||
|
||
|
||
@mock.patch("requests.Session.request") | ||
def test_no_refresh_token_not_dev_mode_enabled(self, mock_post_request): | ||
test_config = {"token": "old_access_token"} | ||
write_new_config_file(**test_config) | ||
mock_post_request.side_effect = [get_response(200, raise_error=False, text=http_response)] | ||
client = Client(config=test_config, config_path=test_config_path, dev_mode=False) | ||
self.assertIsNone(client.refresh_token) | ||
self.assertEqual(client.access_token, "old_access_token") | ||
|
||
@mock.patch("requests.Session.request") | ||
def test_no_refresh_token_dev_mode_enabled(self, mock_post_request): | ||
test_config = {"token": "old_access_token"} | ||
write_new_config_file(**test_config) | ||
mock_post_request.side_effect = [get_response(200, raise_error=False, text=http_response)] | ||
client = Client(config=test_config, config_path=test_config_path, dev_mode=True) | ||
self.assertIsNone(client.refresh_token) | ||
self.assertEqual(client.access_token, "old_access_token") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this import create a conflict with the tap utils.py?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it does because of which I have used an alias here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was causing issue with below import statement in client.py#L8
from tap_typeform.utils import write_config