From a23bd273c29e9f6109a8a843e6376e96fa0c5a7b Mon Sep 17 00:00:00 2001 From: testinnplayin Date: Wed, 26 Aug 2020 14:05:44 +0200 Subject: [PATCH 01/13] feat(oauth): Modified Google Sheets (#190) * feat(googlesheets): beginning of replacing Bearer - CONNECTOR_REGISTRY now returns a Google Sheets connector with auth_flow property - new Google Sheets connector (in addition to old) - No tests yet * feat(googlesheets2): added feature flag - added an empty test * feat(googlesheets2): cleaned out connector * feat(googlesheets2): removed feature flag --- tests/google_sheets_2/test_google_sheets_2.py | 25 ++++++++++++++ toucan_connectors/__init__.py | 7 ++++ toucan_connectors/google_sheets_2/__init__.py | 0 .../google_sheets_2_connector.py | 34 +++++++++++++++++++ toucan_connectors/toucan_connector.py | 2 ++ 5 files changed, 68 insertions(+) create mode 100644 tests/google_sheets_2/test_google_sheets_2.py create mode 100644 toucan_connectors/google_sheets_2/__init__.py create mode 100644 toucan_connectors/google_sheets_2/google_sheets_2_connector.py diff --git a/tests/google_sheets_2/test_google_sheets_2.py b/tests/google_sheets_2/test_google_sheets_2.py new file mode 100644 index 000000000..2399a3765 --- /dev/null +++ b/tests/google_sheets_2/test_google_sheets_2.py @@ -0,0 +1,25 @@ +from pytest import fixture + +from toucan_connectors.google_sheets_2.google_sheets_2_connector import ( + GoogleSheets2Connector, + GoogleSheets2DataSource, +) + + +@fixture +def con(): + return GoogleSheets2Connector(name='test_name', access_token='qweqwe-1111-1111-1111-qweqweqwe') + + +@fixture +def ds(): + return GoogleSheets2DataSource( + name='test_name', + domain='test_domain', + sheet='Constants', + spreadsheet_id='1SMnhnmBm-Tup3SfhS03McCf6S4pS2xqjI6CAXSSBpHU', + ) + + +def test_retrieve_data(): + pass diff --git a/toucan_connectors/__init__.py b/toucan_connectors/__init__.py index 52418b5aa..98c483bba 100644 --- a/toucan_connectors/__init__.py +++ b/toucan_connectors/__init__.py @@ -80,6 +80,11 @@ 'label': 'Google Sheets', 'logo': 'google_sheets/google-sheets.png', }, + 'GoogleSheets2': { + 'connector': 'google_sheets_2.google_sheets_2_connector.GoogleSheets2Connector', + 'label': 'Google Sheets Modified', + 'logo': 'google_sheets/google-sheets.png', + }, 'GoogleSpreadsheet': { 'connector': 'google_spreadsheet.google_spreadsheet_connector.GoogleSpreadsheetConnector', 'label': 'Google Spreadsheet', @@ -188,6 +193,8 @@ def html_base64_image_src(image_path: str) -> str: connector_infos['connector'] = connector_cls with suppress(AttributeError): connector_infos['bearer_integration'] = connector_cls.bearer_integration + with suppress(AttributeError): + connector_infos['auth_flow'] = connector_cls.auth_flow # check if connector implements `get_status`, # which is hence different from `ToucanConnector.get_status` connector_infos['hasStatusCheck'] = ( diff --git a/toucan_connectors/google_sheets_2/__init__.py b/toucan_connectors/google_sheets_2/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/toucan_connectors/google_sheets_2/google_sheets_2_connector.py b/toucan_connectors/google_sheets_2/google_sheets_2_connector.py new file mode 100644 index 000000000..8a9a055f2 --- /dev/null +++ b/toucan_connectors/google_sheets_2/google_sheets_2_connector.py @@ -0,0 +1,34 @@ +"""Google Sheets connector with oauth-manager setup.""" + +# This will replace the old Google Sheets connector that works with the Bearer API +from typing import Optional + +import pandas as pd +from pydantic import Field + +from toucan_connectors.toucan_connector import ToucanConnector, ToucanDataSource + + +class GoogleSheets2DataSource(ToucanDataSource): + spreadsheet_id: str = Field( + ..., + title='ID of the spreadsheet', + description='Can be found in your URL: ' + 'https://docs.google.com/spreadsheets/d//...', + ) + sheet: Optional[str] = Field( + None, title='Sheet title', description='Title of the desired sheet' + ) + header_row: int = Field( + 0, title='Header row', description='Row of the header of the spreadsheet' + ) + + +class GoogleSheets2Connector(ToucanConnector): + data_source_model: GoogleSheets2DataSource + + auth_flow = 'oauth2' + access_token: str + + def _retrieve_data(self, data_source: GoogleSheets2DataSource) -> pd.DataFrame: + pass diff --git a/toucan_connectors/toucan_connector.py b/toucan_connectors/toucan_connector.py index a10913e7f..25ebd12cc 100644 --- a/toucan_connectors/toucan_connector.py +++ b/toucan_connectors/toucan_connector.py @@ -204,6 +204,8 @@ def __init_subclass__(cls): raise TypeError(f'{cls.__name__} has no {e} attribute.') if 'bearer_integration' in cls.__fields__: cls.bearer_integration = cls.__fields__['bearer_integration'].default + if 'auth_flow' in cls.__fields__: + cls.auth_flow = cls.__fields__['auth_flow'].default def bearer_oauth_get_endpoint( self, From 0a46162af1d24e07c8c37ac0f79e17be7f8832fc Mon Sep 17 00:00:00 2001 From: testinnplayin Date: Wed, 2 Sep 2020 09:02:10 +0200 Subject: [PATCH 02/13] feat(googlesheets2): first pass at passing token (#198) added test for codecov --- tests/google_sheets_2/test_google_sheets_2.py | 20 ++++++++++++++++--- .../google_sheets_2_connector.py | 9 +++++++-- toucan_connectors/toucan_connector.py | 5 ++++- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/tests/google_sheets_2/test_google_sheets_2.py b/tests/google_sheets_2/test_google_sheets_2.py index 2399a3765..58d2f002f 100644 --- a/tests/google_sheets_2/test_google_sheets_2.py +++ b/tests/google_sheets_2/test_google_sheets_2.py @@ -8,7 +8,7 @@ @fixture def con(): - return GoogleSheets2Connector(name='test_name', access_token='qweqwe-1111-1111-1111-qweqweqwe') + return GoogleSheets2Connector(name='test_name') @fixture @@ -21,5 +21,19 @@ def ds(): ) -def test_retrieve_data(): - pass +def test__set_secrets(mocker, con): + """It should set secrets on the connector.""" + spy = mocker.spy(GoogleSheets2Connector, 'set_secrets') + fake_secrets = { + 'access_token': 'myaccesstoken', + 'refresh_token': 'myrefreshtoken', + } + con.set_secrets(fake_secrets) + + assert con.secrets == fake_secrets + spy.assert_called_once_with(con, fake_secrets) + + +def test_retrieve_data(con, ds): + """It should just work for now.""" + con._retrieve_data(ds) diff --git a/toucan_connectors/google_sheets_2/google_sheets_2_connector.py b/toucan_connectors/google_sheets_2/google_sheets_2_connector.py index 8a9a055f2..b7faa3049 100644 --- a/toucan_connectors/google_sheets_2/google_sheets_2_connector.py +++ b/toucan_connectors/google_sheets_2/google_sheets_2_connector.py @@ -1,7 +1,7 @@ """Google Sheets connector with oauth-manager setup.""" # This will replace the old Google Sheets connector that works with the Bearer API -from typing import Optional +from typing import Dict, Optional import pandas as pd from pydantic import Field @@ -28,7 +28,12 @@ class GoogleSheets2Connector(ToucanConnector): data_source_model: GoogleSheets2DataSource auth_flow = 'oauth2' - access_token: str + + secrets: Optional[Dict[str, str]] + + def set_secrets(self, secrets: Dict[str, str]): + """Set the secrets from inside the main service.""" + self.secrets = secrets def _retrieve_data(self, data_source: GoogleSheets2DataSource) -> pd.DataFrame: pass diff --git a/toucan_connectors/toucan_connector.py b/toucan_connectors/toucan_connector.py index 25ebd12cc..0990f0bb5 100644 --- a/toucan_connectors/toucan_connector.py +++ b/toucan_connectors/toucan_connector.py @@ -232,13 +232,16 @@ def _retrieve_data(self, data_source: ToucanDataSource): @decorate_func_with_retry def get_df( - self, data_source: ToucanDataSource, permissions: Optional[dict] = None + self, + data_source: ToucanDataSource, + permissions: Optional[dict] = None, ) -> pd.DataFrame: """ Method to retrieve the data as a pandas dataframe filtered by permissions """ res = self._retrieve_data(data_source) + if permissions is not None: permissions_query = PandasConditionTranslator.translate(permissions) permissions_query = apply_query_parameters(permissions_query, data_source.parameters) From 1a363aa332a0410086a322b77357f9dcd3744ea9 Mon Sep 17 00:00:00 2001 From: testinnplayin Date: Fri, 28 Aug 2020 17:36:27 +0200 Subject: [PATCH 03/13] feat(googlesheets2): using tokens in connector --- tests/google_sheets_2/test_google_sheets_2.py | 91 ++++++++++++++++++- tests/test_common.py | 40 ++++++++ toucan_connectors/common.py | 11 +++ .../google_sheets_2_connector.py | 87 +++++++++++++++++- 4 files changed, 224 insertions(+), 5 deletions(-) diff --git a/tests/google_sheets_2/test_google_sheets_2.py b/tests/google_sheets_2/test_google_sheets_2.py index 58d2f002f..2fd79b1c7 100644 --- a/tests/google_sheets_2/test_google_sheets_2.py +++ b/tests/google_sheets_2/test_google_sheets_2.py @@ -1,10 +1,18 @@ +import pytest +from aiohttp import web from pytest import fixture +import tests.general_helpers as helpers from toucan_connectors.google_sheets_2.google_sheets_2_connector import ( GoogleSheets2Connector, GoogleSheets2DataSource, + get_data, + run_fetch, ) +import_path = 'toucan_connectors.google_sheets_2.google_sheets_2_connector' +run_fetch_fn = f'{import_path}.run_fetch' + @fixture def con(): @@ -21,7 +29,7 @@ def ds(): ) -def test__set_secrets(mocker, con): +def test_set_secrets(mocker, con): """It should set secrets on the connector.""" spy = mocker.spy(GoogleSheets2Connector, 'set_secrets') fake_secrets = { @@ -34,6 +42,81 @@ def test__set_secrets(mocker, con): spy.assert_called_once_with(con, fake_secrets) -def test_retrieve_data(con, ds): - """It should just work for now.""" - con._retrieve_data(ds) +FAKE_SPREADSHEET = { + 'metadata': '...', + 'values': [['country', 'city'], ['France', 'Paris'], ['England', 'London']], +} + + +def test_spreadsheet_success(mocker, con, ds): + """It should return a spreadsheet.""" + con.set_secrets( + { + 'access_token': 'myaccesstoken', + 'refresh_token': 'myrefreshtoken', + } + ) + + mocker.patch(run_fetch_fn, return_value=FAKE_SPREADSHEET) + + df = con.get_df(ds) + + assert df.shape == (2, 2) + assert df.columns.tolist() == ['country', 'city'] + + ds.header_row = 1 + df = con.get_df(ds) + assert df.shape == (1, 2) + assert df.columns.tolist() == ['France', 'Paris'] + + +def test_spreadsheet_no_secrets(mocker, con, ds): + """It should raise an exception if there no secrets passed or no access token.""" + mocker.patch(run_fetch_fn, return_value=FAKE_SPREADSHEET) + + with pytest.raises(Exception) as err: + con.get_df(ds) + + assert str(err.value) == 'No credentials' + + con.set_secrets({'refresh_token': 'myrefreshtoken'}) + + with pytest.raises(KeyError): + con.get_df(ds) + + +def test_set_columns(mocker, con, ds): + """It should return a well-formed column set.""" + con.set_secrets({'access_token': 'foo', 'refresh_token': 'bar'}) + fake_results = { + 'metadata': '...', + 'values': [['Animateur', '', '', 'Week'], ['pika', '', 'a', 'W1'], ['bulbi', '', '', 'W2']], + } + mocker.patch(run_fetch_fn, return_value=fake_results) + + df = con.get_df(ds) + assert df.to_dict() == { + 'Animateur': {1: 'pika', 2: 'bulbi'}, + 1: {1: '', 2: ''}, + 2: {1: 'a', 2: ''}, + 'Week': {1: 'W1', 2: 'W2'}, + } + + +def test_run_fetch(mocker): + """It should return a result from loops if all is ok.""" + mocker.patch(f'{import_path}.get_data', return_value=helpers.build_future(FAKE_SPREADSHEET)) + + result = run_fetch('/fudge', 'myaccesstoken') + + assert result == FAKE_SPREADSHEET + + +@pytest.mark.asyncio +async def test_get_data(mocker): + """It should return a result from fetch if all is ok.""" + mocker.patch(f'{import_path}.fetch', return_value=helpers.build_future(FAKE_SPREADSHEET)) + + result = await get_data('/foo', 'myaccesstoken') + + assert result == FAKE_SPREADSHEET diff --git a/tests/test_common.py b/tests/test_common.py index 2baf4b7b3..b4da5f5d2 100644 --- a/tests/test_common.py +++ b/tests/test_common.py @@ -1,8 +1,10 @@ import pytest +from aiohttp import web from toucan_connectors.common import ( NonValidVariable, apply_query_parameters, + fetch, nosql_apply_parameters_to_query, ) @@ -183,3 +185,41 @@ def test_bad_variable_in_query(): with pytest.raises(NonValidVariable) as err: nosql_apply_parameters_to_query(query, params, handle_errors=True) assert str(err.value) == 'Non valid variable thing' + + +# fetch tests + +FAKE_DATA = {'foo': 'bar', 'baz': 'fudge'} + + +async def send_200_success(req: web.Request): + """Send a response with a success.""" + return web.json_response(FAKE_DATA, status=200) + + +async def send_401_error(req: web.Request) -> dict: + """Send a response with an error.""" + return web.Response(reason='Unauthorized', status=401) + + +async def test_fetch_happy(aiohttp_client, loop): + """It should return a properly-formed dictionary.""" + app = web.Application(loop=loop) + app.router.add_get('/foo', send_200_success) + + client = await aiohttp_client(app) + res = await fetch('/foo', client) + + assert res == FAKE_DATA + + +async def test_fetch_bad_response(aiohttp_client, loop): + """It should throw an Exception with a message if there is an error.""" + app = web.Application(loop=loop) + app.router.add_get('/hotels', send_401_error) + + client = await aiohttp_client(app) + with pytest.raises(Exception) as err: + await fetch('/hotels', client) + + assert str(err.value) == 'Aborting request due to error from the API: 401, Unauthorized' diff --git a/toucan_connectors/common.py b/toucan_connectors/common.py index 704421937..575d42309 100644 --- a/toucan_connectors/common.py +++ b/toucan_connectors/common.py @@ -4,6 +4,7 @@ from copy import deepcopy import pyjq +from aiohttp import ClientSession from jinja2 import Environment, StrictUndefined, Template, meta from pydantic import Field from toucan_data_sdk.utils.helpers import slugify @@ -204,3 +205,13 @@ def get_loop(): asyncio.set_event_loop(loop) return loop + + +async def fetch(url: str, session: ClientSession): + """Fetch data from an API.""" + async with session.get(url) as res: + if res.status != 200: + raise Exception( + f'Aborting request due to error from the API: {res.status}, {res.reason}' + ) + return await res.json() diff --git a/toucan_connectors/google_sheets_2/google_sheets_2_connector.py b/toucan_connectors/google_sheets_2/google_sheets_2_connector.py index b7faa3049..453f7a9d6 100644 --- a/toucan_connectors/google_sheets_2/google_sheets_2_connector.py +++ b/toucan_connectors/google_sheets_2/google_sheets_2_connector.py @@ -1,15 +1,42 @@ """Google Sheets connector with oauth-manager setup.""" # This will replace the old Google Sheets connector that works with the Bearer API +import asyncio from typing import Dict, Optional import pandas as pd +from aiohttp import ClientSession from pydantic import Field +from toucan_connectors.common import fetch, get_loop from toucan_connectors.toucan_connector import ToucanConnector, ToucanDataSource +async def get_data(url, access_token): + """Build the final request along with headers.""" + headers = {'Authorization': f'Bearer {access_token}'} + + async with ClientSession(headers=headers) as session: + return await fetch(url, session) + + +def run_fetch(url, access_token): + """Run loop.""" + loop = get_loop() + future = asyncio.ensure_future(get_data(url, access_token)) + return loop.run_until_complete(future) + + class GoogleSheets2DataSource(ToucanDataSource): + """ + Google Spreadsheet 2 data source class. + + Contains: + - spreadsheet_id + - sheet + - header_row + """ + spreadsheet_id: str = Field( ..., title='ID of the spreadsheet', @@ -23,12 +50,34 @@ class GoogleSheets2DataSource(ToucanDataSource): 0, title='Header row', description='Row of the header of the spreadsheet' ) + @classmethod + def get_form(cls, connector: 'GoogleSheets2Connector', current_config): + """Retrieve a form filled with suggestions of available sheets.""" + # Always add the suggestions for the available sheets + baseroute = 'https://sheets.googleapis.com/v4/spreadsheets/' + constraints = {} + with suppress(Exception): + partial_endpoint = current_config['spreadsheet_id'] + final_url = f'{self.baseroute}/{partial_endpoint}' + data = run_fetch(final_url, connector.access_token) + # data = connector.bearer_oauth_get_endpoint(current_config['spreadsheet_id']) + available_sheets = [str(x['properties']['title']) for x in data['sheets']] + constraints['sheet'] = strlist_to_enum('sheet', available_sheets) + + return create_model('FormSchema', **constraints, __base__=cls).schema() + class GoogleSheets2Connector(ToucanConnector): + """The Google Sheets connector.""" + data_source_model: GoogleSheets2DataSource auth_flow = 'oauth2' + # The following should be hidden properties + + baseroute = 'https://sheets.googleapis.com/v4/spreadsheets/' + secrets: Optional[Dict[str, str]] def set_secrets(self, secrets: Dict[str, str]): @@ -36,4 +85,40 @@ def set_secrets(self, secrets: Dict[str, str]): self.secrets = secrets def _retrieve_data(self, data_source: GoogleSheets2DataSource) -> pd.DataFrame: - pass + """ + Point of entry for data retrieval in the connector + + Requires: + - Datasource + """ + if not self.secrets: + raise Exception('No credentials') + + access_token = self.secrets['access_token'] + + if data_source.sheet is None: + # Get spreadsheet informations and retrieve all the available sheets + # https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/get + data = run_fetch(self.baseroute, access_token) + available_sheets = [str(x['properties']['title']) for x in data['sheets']] + data_source.sheet = available_sheets[0] + + # https://developers.google.com/sheets/api/samples/reading + read_sheet_endpoint = f'{data_source.spreadsheet_id}/values/{data_source.sheet}' + full_url = f'{self.baseroute}/{read_sheet_endpoint}/values/{data_source.sheet}' + data = run_fetch(full_url, access_token)['values'] + df = pd.DataFrame(data) + + # Since `data` is a list of lists, the columns are not set properly + # df = + # 0 1 2 + # 0 animateur week + # 1 pika W1 + # 2 bulbi W2 + # + # We set the first row as the header by default and replace empty value by the index + # to avoid having errors when trying to jsonify it (two columns can't have the same value) + df.columns = [name or index for index, name in enumerate(df.iloc[data_source.header_row])] + df = df[data_source.header_row + 1 :] + + return df From 7d58ece2813e3a7595d182522ab58d357029ed21 Mon Sep 17 00:00:00 2001 From: testinnplayin Date: Wed, 2 Sep 2020 11:31:03 +0200 Subject: [PATCH 04/13] feat(googlesheets2): added new test for get_form --- doc/connectors/google_sheets_2.md | 1 + tests/google_sheets_2/test_google_sheets_2.py | 69 +++++++++++++++---- .../google_sheets_2_connector.py | 43 ++++++------ 3 files changed, 78 insertions(+), 35 deletions(-) create mode 100644 doc/connectors/google_sheets_2.md diff --git a/doc/connectors/google_sheets_2.md b/doc/connectors/google_sheets_2.md new file mode 100644 index 000000000..4fd640e1d --- /dev/null +++ b/doc/connectors/google_sheets_2.md @@ -0,0 +1 @@ +# This is the doc for the Google Sheets 2 connector \ No newline at end of file diff --git a/tests/google_sheets_2/test_google_sheets_2.py b/tests/google_sheets_2/test_google_sheets_2.py index 2fd79b1c7..aa7cd553e 100644 --- a/tests/google_sheets_2/test_google_sheets_2.py +++ b/tests/google_sheets_2/test_google_sheets_2.py @@ -1,17 +1,15 @@ import pytest -from aiohttp import web from pytest import fixture import tests.general_helpers as helpers from toucan_connectors.google_sheets_2.google_sheets_2_connector import ( GoogleSheets2Connector, GoogleSheets2DataSource, - get_data, - run_fetch, ) import_path = 'toucan_connectors.google_sheets_2.google_sheets_2_connector' -run_fetch_fn = f'{import_path}.run_fetch' + +python_version_is_older = helpers.check_py_version((3, 8)) @fixture @@ -29,6 +27,51 @@ def ds(): ) +FAKE_SHEET_LIST_RESPONSE = { + 'sheets': [ + { + 'properties': {'title': 'Foo'}, + }, + { + 'properties': {'title': 'Bar'}, + }, + { + 'properties': {'title': 'Baz'}, + }, + ] +} + + +def test_get_form_with_secrets(mocker, con, ds): + """It should return a list of spreadsheet titles.""" + con.set_secrets({'access_token': 'foo'}) + mocker.patch.object(GoogleSheets2Connector, '_run_fetch', return_value=FAKE_SHEET_LIST_RESPONSE) + + result = ds.get_form( + connector=con, + current_config={'spreadsheet_id': '1SMnhnmBm-Tup3SfhS03McCf6S4pS2xqjI6CAXSSBpHU'}, + ) + print('result ', result) + expected_results = ['Foo', 'Bar', 'Baz'] + if python_version_is_older: + assert result['definitions']['sheet']['enum'] == expected_results + else: + assert result['properties']['sheet']['enum'] == expected_results + + +def test_get_form_no_secrets(mocker, con, ds): + """It should return no spreadsheet titles.""" + mocker.patch.object(GoogleSheets2Connector, '_run_fetch', return_value=Exception) + result = ds.get_form( + connector=con, + current_config={'spreadsheet_id': '1SMnhnmBm-Tup3SfhS03McCf6S4pS2xqjI6CAXSSBpHU'}, + ) + if python_version_is_older: + assert not result.get('definitions') + else: + assert not result['properties']['sheet'].get('enum') + + def test_set_secrets(mocker, con): """It should set secrets on the connector.""" spy = mocker.spy(GoogleSheets2Connector, 'set_secrets') @@ -57,7 +100,7 @@ def test_spreadsheet_success(mocker, con, ds): } ) - mocker.patch(run_fetch_fn, return_value=FAKE_SPREADSHEET) + mocker.patch.object(GoogleSheets2Connector, '_run_fetch', return_value=FAKE_SPREADSHEET) df = con.get_df(ds) @@ -72,7 +115,7 @@ def test_spreadsheet_success(mocker, con, ds): def test_spreadsheet_no_secrets(mocker, con, ds): """It should raise an exception if there no secrets passed or no access token.""" - mocker.patch(run_fetch_fn, return_value=FAKE_SPREADSHEET) + mocker.patch.object(GoogleSheets2Connector, '_run_fetch', return_value=FAKE_SPREADSHEET) with pytest.raises(Exception) as err: con.get_df(ds) @@ -92,7 +135,7 @@ def test_set_columns(mocker, con, ds): 'metadata': '...', 'values': [['Animateur', '', '', 'Week'], ['pika', '', 'a', 'W1'], ['bulbi', '', '', 'W2']], } - mocker.patch(run_fetch_fn, return_value=fake_results) + mocker.patch.object(GoogleSheets2Connector, '_run_fetch', return_value=fake_results) df = con.get_df(ds) assert df.to_dict() == { @@ -103,20 +146,22 @@ def test_set_columns(mocker, con, ds): } -def test_run_fetch(mocker): +def test__run_fetch(mocker, con): """It should return a result from loops if all is ok.""" - mocker.patch(f'{import_path}.get_data', return_value=helpers.build_future(FAKE_SPREADSHEET)) + mocker.patch.object( + GoogleSheets2Connector, '_get_data', return_value=helpers.build_future(FAKE_SPREADSHEET) + ) - result = run_fetch('/fudge', 'myaccesstoken') + result = con._run_fetch('/fudge', 'myaccesstoken') assert result == FAKE_SPREADSHEET @pytest.mark.asyncio -async def test_get_data(mocker): +async def test_get_data(mocker, con): """It should return a result from fetch if all is ok.""" mocker.patch(f'{import_path}.fetch', return_value=helpers.build_future(FAKE_SPREADSHEET)) - result = await get_data('/foo', 'myaccesstoken') + result = await con._get_data('/foo', 'myaccesstoken') assert result == FAKE_SPREADSHEET diff --git a/toucan_connectors/google_sheets_2/google_sheets_2_connector.py b/toucan_connectors/google_sheets_2/google_sheets_2_connector.py index 453f7a9d6..af6de4ae3 100644 --- a/toucan_connectors/google_sheets_2/google_sheets_2_connector.py +++ b/toucan_connectors/google_sheets_2/google_sheets_2_connector.py @@ -2,29 +2,15 @@ # This will replace the old Google Sheets connector that works with the Bearer API import asyncio +from contextlib import suppress from typing import Dict, Optional import pandas as pd from aiohttp import ClientSession -from pydantic import Field +from pydantic import Field, create_model from toucan_connectors.common import fetch, get_loop -from toucan_connectors.toucan_connector import ToucanConnector, ToucanDataSource - - -async def get_data(url, access_token): - """Build the final request along with headers.""" - headers = {'Authorization': f'Bearer {access_token}'} - - async with ClientSession(headers=headers) as session: - return await fetch(url, session) - - -def run_fetch(url, access_token): - """Run loop.""" - loop = get_loop() - future = asyncio.ensure_future(get_data(url, access_token)) - return loop.run_until_complete(future) +from toucan_connectors.toucan_connector import ToucanConnector, ToucanDataSource, strlist_to_enum class GoogleSheets2DataSource(ToucanDataSource): @@ -54,13 +40,11 @@ class GoogleSheets2DataSource(ToucanDataSource): def get_form(cls, connector: 'GoogleSheets2Connector', current_config): """Retrieve a form filled with suggestions of available sheets.""" # Always add the suggestions for the available sheets - baseroute = 'https://sheets.googleapis.com/v4/spreadsheets/' constraints = {} with suppress(Exception): partial_endpoint = current_config['spreadsheet_id'] - final_url = f'{self.baseroute}/{partial_endpoint}' - data = run_fetch(final_url, connector.access_token) - # data = connector.bearer_oauth_get_endpoint(current_config['spreadsheet_id']) + final_url = f'{connector.baseroute}{partial_endpoint}' + data = connector._run_fetch(final_url, connector.secrets['access_token']) available_sheets = [str(x['properties']['title']) for x in data['sheets']] constraints['sheet'] = strlist_to_enum('sheet', available_sheets) @@ -80,10 +64,23 @@ class GoogleSheets2Connector(ToucanConnector): secrets: Optional[Dict[str, str]] + async def _get_data(self, url, access_token): + """Build the final request along with headers.""" + headers = {'Authorization': f'Bearer {access_token}'} + + async with ClientSession(headers=headers) as session: + return await fetch(url, session) + def set_secrets(self, secrets: Dict[str, str]): """Set the secrets from inside the main service.""" self.secrets = secrets + def _run_fetch(self, url, access_token): + """Run loop.""" + loop = get_loop() + future = asyncio.ensure_future(self._get_data(url, access_token)) + return loop.run_until_complete(future) + def _retrieve_data(self, data_source: GoogleSheets2DataSource) -> pd.DataFrame: """ Point of entry for data retrieval in the connector @@ -99,14 +96,14 @@ def _retrieve_data(self, data_source: GoogleSheets2DataSource) -> pd.DataFrame: if data_source.sheet is None: # Get spreadsheet informations and retrieve all the available sheets # https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/get - data = run_fetch(self.baseroute, access_token) + data = self._run_fetch(self.baseroute, access_token) available_sheets = [str(x['properties']['title']) for x in data['sheets']] data_source.sheet = available_sheets[0] # https://developers.google.com/sheets/api/samples/reading read_sheet_endpoint = f'{data_source.spreadsheet_id}/values/{data_source.sheet}' full_url = f'{self.baseroute}/{read_sheet_endpoint}/values/{data_source.sheet}' - data = run_fetch(full_url, access_token)['values'] + data = self._run_fetch(full_url, access_token)['values'] df = pd.DataFrame(data) # Since `data` is a list of lists, the columns are not set properly From ecb8c036854e4baa58834598dbbc240575a8448b Mon Sep 17 00:00:00 2001 From: testinnplayin Date: Wed, 2 Sep 2020 17:29:17 +0200 Subject: [PATCH 05/13] feat(googlesheets2): added documentation --- doc/connectors/google_sheets_2.md | 48 ++++++++++++++++++- tests/google_sheets_2/test_google_sheets_2.py | 33 ++++++------- 2 files changed, 64 insertions(+), 17 deletions(-) diff --git a/doc/connectors/google_sheets_2.md b/doc/connectors/google_sheets_2.md index 4fd640e1d..77bd70ad7 100644 --- a/doc/connectors/google_sheets_2.md +++ b/doc/connectors/google_sheets_2.md @@ -1 +1,47 @@ -# This is the doc for the Google Sheets 2 connector \ No newline at end of file +# This is the doc for the Google Sheets 2 connector + +## Data provider configuration + +* `type`: `"GoogleSheets2"` +* `name`: str, required +* `auth_flow`: str +* `baseroute`: str +* `secrets`: dict + +The `auth_flow` property marks this as being a connector that uses the connector_oauth_manager for the oauth dance. + +The `baseroute` is fixed and is 'https://sheets.googleapis.com/v4/spreadsheets/'. + +The `secrets` dictionary contains the `access_token` and a `refresh_token` (if there is one). Though `secrets` is optional during the initial creation of the connector, it is necessary for when the user wants to make requests to the connector. If there is no `access_token`, an Exception is thrown. + + +```coffee +DATA_PROVIDERS: [ + type: 'GoogleSheets' + name: '' +, + ... +] +``` + +## Data source configuration + +* `domain`: str, required +* `name`: str, required. Should match the data provider name +* `spreadsheet_id`: str, required. Id of the spreadsheet which can be found inside +the url: https://docs.google.com/spreadsheets/d//edit?pref=2&pli=1#gid=0, +* `sheet`: str. By default, the extractor returns the first sheet. +* `header_row`: int, default to 0. Row of the header of the spreadsheet + + +```coffee +DATA_SOURCES: [ + domain: '' + name: '' + spreadsheet_id: '' + sheetname: '' + skip_rows: +, + ... +] +``` \ No newline at end of file diff --git a/tests/google_sheets_2/test_google_sheets_2.py b/tests/google_sheets_2/test_google_sheets_2.py index aa7cd553e..ebf5b5324 100644 --- a/tests/google_sheets_2/test_google_sheets_2.py +++ b/tests/google_sheets_2/test_google_sheets_2.py @@ -27,6 +27,22 @@ def ds(): ) +FAKE_SPREADSHEET = { + 'metadata': '...', + 'values': [['country', 'city'], ['France', 'Paris'], ['England', 'London']], +} + + +@pytest.mark.asyncio +async def test_get_data(mocker, con): + """It should return a result from fetch if all is ok.""" + mocker.patch(f'{import_path}.fetch', return_value=helpers.build_future(FAKE_SPREADSHEET)) + + result = await con._get_data('/foo', 'myaccesstoken') + + assert result == FAKE_SPREADSHEET + + FAKE_SHEET_LIST_RESPONSE = { 'sheets': [ { @@ -85,12 +101,6 @@ def test_set_secrets(mocker, con): spy.assert_called_once_with(con, fake_secrets) -FAKE_SPREADSHEET = { - 'metadata': '...', - 'values': [['country', 'city'], ['France', 'Paris'], ['England', 'London']], -} - - def test_spreadsheet_success(mocker, con, ds): """It should return a spreadsheet.""" con.set_secrets( @@ -146,6 +156,7 @@ def test_set_columns(mocker, con, ds): } +@pytest.mark.skip(reason='Update seems to have broken this test') def test__run_fetch(mocker, con): """It should return a result from loops if all is ok.""" mocker.patch.object( @@ -155,13 +166,3 @@ def test__run_fetch(mocker, con): result = con._run_fetch('/fudge', 'myaccesstoken') assert result == FAKE_SPREADSHEET - - -@pytest.mark.asyncio -async def test_get_data(mocker, con): - """It should return a result from fetch if all is ok.""" - mocker.patch(f'{import_path}.fetch', return_value=helpers.build_future(FAKE_SPREADSHEET)) - - result = await con._get_data('/foo', 'myaccesstoken') - - assert result == FAKE_SPREADSHEET From da22b3b1098cedd76311eb425e963ffb69cbab46 Mon Sep 17 00:00:00 2001 From: testinnplayin Date: Wed, 2 Sep 2020 09:02:10 +0200 Subject: [PATCH 06/13] feat(googlesheets2): first pass at passing token (#198) added test for codecov fixed test --- tests/google_sheets_2/test_google_sheets_2.py | 11 ++++------- .../google_sheets_2/google_sheets_2_connector.py | 4 ++++ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/google_sheets_2/test_google_sheets_2.py b/tests/google_sheets_2/test_google_sheets_2.py index ebf5b5324..bb04d6ea6 100644 --- a/tests/google_sheets_2/test_google_sheets_2.py +++ b/tests/google_sheets_2/test_google_sheets_2.py @@ -9,8 +9,6 @@ import_path = 'toucan_connectors.google_sheets_2.google_sheets_2_connector' -python_version_is_older = helpers.check_py_version((3, 8)) - @fixture def con(): @@ -67,9 +65,8 @@ def test_get_form_with_secrets(mocker, con, ds): connector=con, current_config={'spreadsheet_id': '1SMnhnmBm-Tup3SfhS03McCf6S4pS2xqjI6CAXSSBpHU'}, ) - print('result ', result) expected_results = ['Foo', 'Bar', 'Baz'] - if python_version_is_older: + if result.get('definitions'): assert result['definitions']['sheet']['enum'] == expected_results else: assert result['properties']['sheet']['enum'] == expected_results @@ -82,10 +79,10 @@ def test_get_form_no_secrets(mocker, con, ds): connector=con, current_config={'spreadsheet_id': '1SMnhnmBm-Tup3SfhS03McCf6S4pS2xqjI6CAXSSBpHU'}, ) - if python_version_is_older: - assert not result.get('definitions') - else: + if result.get('properties'): assert not result['properties']['sheet'].get('enum') + else: + assert not result.get('definitions') def test_set_secrets(mocker, con): diff --git a/toucan_connectors/google_sheets_2/google_sheets_2_connector.py b/toucan_connectors/google_sheets_2/google_sheets_2_connector.py index af6de4ae3..a4027b818 100644 --- a/toucan_connectors/google_sheets_2/google_sheets_2_connector.py +++ b/toucan_connectors/google_sheets_2/google_sheets_2_connector.py @@ -81,6 +81,10 @@ def _run_fetch(self, url, access_token): future = asyncio.ensure_future(self._get_data(url, access_token)) return loop.run_until_complete(future) + def set_secrets(self, secrets: Dict[str, str]): + """Set the secrets from inside the main service.""" + self.secrets = secrets + def _retrieve_data(self, data_source: GoogleSheets2DataSource) -> pd.DataFrame: """ Point of entry for data retrieval in the connector From 342403f9782a5b443e0dcacc9e81d166b9137562 Mon Sep 17 00:00:00 2001 From: testinnplayin Date: Thu, 3 Sep 2020 16:15:32 +0200 Subject: [PATCH 07/13] feat(googlesheets2): fixed url --- .../google_sheets_2/google_sheets_2_connector.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/toucan_connectors/google_sheets_2/google_sheets_2_connector.py b/toucan_connectors/google_sheets_2/google_sheets_2_connector.py index a4027b818..f69878122 100644 --- a/toucan_connectors/google_sheets_2/google_sheets_2_connector.py +++ b/toucan_connectors/google_sheets_2/google_sheets_2_connector.py @@ -3,7 +3,7 @@ # This will replace the old Google Sheets connector that works with the Bearer API import asyncio from contextlib import suppress -from typing import Dict, Optional +from typing import Any, Dict, Optional import pandas as pd from aiohttp import ClientSession @@ -62,7 +62,7 @@ class GoogleSheets2Connector(ToucanConnector): baseroute = 'https://sheets.googleapis.com/v4/spreadsheets/' - secrets: Optional[Dict[str, str]] + secrets: Optional[Dict[str, Any]] async def _get_data(self, url, access_token): """Build the final request along with headers.""" @@ -106,7 +106,7 @@ def _retrieve_data(self, data_source: GoogleSheets2DataSource) -> pd.DataFrame: # https://developers.google.com/sheets/api/samples/reading read_sheet_endpoint = f'{data_source.spreadsheet_id}/values/{data_source.sheet}' - full_url = f'{self.baseroute}/{read_sheet_endpoint}/values/{data_source.sheet}' + full_url = f'{self.baseroute}{read_sheet_endpoint}' data = self._run_fetch(full_url, access_token)['values'] df = pd.DataFrame(data) From ccd96e1003d71ac17313f52e0aebdef2141df6df Mon Sep 17 00:00:00 2001 From: testinnplayin Date: Thu, 3 Sep 2020 15:33:35 +0200 Subject: [PATCH 08/13] feat(googlesheets2): using tokens in requests (#200) --- .../google_sheets_2/google_sheets_2_connector.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/toucan_connectors/google_sheets_2/google_sheets_2_connector.py b/toucan_connectors/google_sheets_2/google_sheets_2_connector.py index f69878122..37a3fb68c 100644 --- a/toucan_connectors/google_sheets_2/google_sheets_2_connector.py +++ b/toucan_connectors/google_sheets_2/google_sheets_2_connector.py @@ -81,10 +81,23 @@ def _run_fetch(self, url, access_token): future = asyncio.ensure_future(self._get_data(url, access_token)) return loop.run_until_complete(future) + async def _get_data(self, url, access_token): + """Build the final request along with headers.""" + headers = {'Authorization': f'Bearer {access_token}'} + + async with ClientSession(headers=headers) as session: + return await fetch(url, session) + def set_secrets(self, secrets: Dict[str, str]): """Set the secrets from inside the main service.""" self.secrets = secrets + def _run_fetch(self, url, access_token): + """Run loop.""" + loop = get_loop() + future = asyncio.ensure_future(self._get_data(url, access_token)) + return loop.run_until_complete(future) + def _retrieve_data(self, data_source: GoogleSheets2DataSource) -> pd.DataFrame: """ Point of entry for data retrieval in the connector @@ -107,6 +120,7 @@ def _retrieve_data(self, data_source: GoogleSheets2DataSource) -> pd.DataFrame: # https://developers.google.com/sheets/api/samples/reading read_sheet_endpoint = f'{data_source.spreadsheet_id}/values/{data_source.sheet}' full_url = f'{self.baseroute}{read_sheet_endpoint}' + data = self._run_fetch(full_url, access_token)['values'] df = pd.DataFrame(data) From 992966dc0a7d49f6122685e34a3ef7e1726a2f5d Mon Sep 17 00:00:00 2001 From: David Nowinsky Date: Fri, 4 Sep 2020 09:19:35 +0200 Subject: [PATCH 09/13] chore: remove duplicate function in GSheets connector --- .../google_sheets_2_connector.py | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/toucan_connectors/google_sheets_2/google_sheets_2_connector.py b/toucan_connectors/google_sheets_2/google_sheets_2_connector.py index 37a3fb68c..b2efcaa2f 100644 --- a/toucan_connectors/google_sheets_2/google_sheets_2_connector.py +++ b/toucan_connectors/google_sheets_2/google_sheets_2_connector.py @@ -64,23 +64,6 @@ class GoogleSheets2Connector(ToucanConnector): secrets: Optional[Dict[str, Any]] - async def _get_data(self, url, access_token): - """Build the final request along with headers.""" - headers = {'Authorization': f'Bearer {access_token}'} - - async with ClientSession(headers=headers) as session: - return await fetch(url, session) - - def set_secrets(self, secrets: Dict[str, str]): - """Set the secrets from inside the main service.""" - self.secrets = secrets - - def _run_fetch(self, url, access_token): - """Run loop.""" - loop = get_loop() - future = asyncio.ensure_future(self._get_data(url, access_token)) - return loop.run_until_complete(future) - async def _get_data(self, url, access_token): """Build the final request along with headers.""" headers = {'Authorization': f'Bearer {access_token}'} From 8e982d926ebd60d42377168675d7137fd8de51b0 Mon Sep 17 00:00:00 2001 From: David Nowinsky Date: Fri, 4 Sep 2020 12:37:35 +0200 Subject: [PATCH 10/13] fix(gsheets2): fix fetching of sheets list and test it --- tests/google_sheets_2/test_google_sheets_2.py | 86 +++++++++++++------ .../google_sheets_2_connector.py | 2 +- 2 files changed, 63 insertions(+), 25 deletions(-) diff --git a/tests/google_sheets_2/test_google_sheets_2.py b/tests/google_sheets_2/test_google_sheets_2.py index bb04d6ea6..acea6a93c 100644 --- a/tests/google_sheets_2/test_google_sheets_2.py +++ b/tests/google_sheets_2/test_google_sheets_2.py @@ -1,3 +1,5 @@ +from unittest.mock import Mock + import pytest from pytest import fixture @@ -15,6 +17,12 @@ def con(): return GoogleSheets2Connector(name='test_name') +@fixture +def con_with_secrets(con): + con.set_secrets({'access_token': 'foo', 'refresh_token': 'bar'}) + return con + + @fixture def ds(): return GoogleSheets2DataSource( @@ -25,7 +33,16 @@ def ds(): ) -FAKE_SPREADSHEET = { +@fixture +def ds_without_sheet(): + return GoogleSheets2DataSource( + name='test_name', + domain='test_domain', + spreadsheet_id='1SMnhnmBm-Tup3SfhS03McCf6S4pS2xqjI6CAXSSBpHU', + ) + + +FAKE_SHEET = { 'metadata': '...', 'values': [['country', 'city'], ['France', 'Paris'], ['England', 'London']], } @@ -34,11 +51,11 @@ def ds(): @pytest.mark.asyncio async def test_get_data(mocker, con): """It should return a result from fetch if all is ok.""" - mocker.patch(f'{import_path}.fetch', return_value=helpers.build_future(FAKE_SPREADSHEET)) + mocker.patch(f'{import_path}.fetch', return_value=helpers.build_future(FAKE_SHEET)) result = await con._get_data('/foo', 'myaccesstoken') - assert result == FAKE_SPREADSHEET + assert result == FAKE_SHEET FAKE_SHEET_LIST_RESPONSE = { @@ -56,13 +73,12 @@ async def test_get_data(mocker, con): } -def test_get_form_with_secrets(mocker, con, ds): +def test_get_form_with_secrets(mocker, con_with_secrets, ds): """It should return a list of spreadsheet titles.""" - con.set_secrets({'access_token': 'foo'}) mocker.patch.object(GoogleSheets2Connector, '_run_fetch', return_value=FAKE_SHEET_LIST_RESPONSE) result = ds.get_form( - connector=con, + connector=con_with_secrets, current_config={'spreadsheet_id': '1SMnhnmBm-Tup3SfhS03McCf6S4pS2xqjI6CAXSSBpHU'}, ) expected_results = ['Foo', 'Bar', 'Baz'] @@ -98,31 +114,24 @@ def test_set_secrets(mocker, con): spy.assert_called_once_with(con, fake_secrets) -def test_spreadsheet_success(mocker, con, ds): +def test_spreadsheet_success(mocker, con_with_secrets, ds): """It should return a spreadsheet.""" - con.set_secrets( - { - 'access_token': 'myaccesstoken', - 'refresh_token': 'myrefreshtoken', - } - ) - - mocker.patch.object(GoogleSheets2Connector, '_run_fetch', return_value=FAKE_SPREADSHEET) + mocker.patch.object(GoogleSheets2Connector, '_run_fetch', return_value=FAKE_SHEET) - df = con.get_df(ds) + df = con_with_secrets.get_df(ds) assert df.shape == (2, 2) assert df.columns.tolist() == ['country', 'city'] ds.header_row = 1 - df = con.get_df(ds) + df = con_with_secrets.get_df(ds) assert df.shape == (1, 2) assert df.columns.tolist() == ['France', 'Paris'] def test_spreadsheet_no_secrets(mocker, con, ds): """It should raise an exception if there no secrets passed or no access token.""" - mocker.patch.object(GoogleSheets2Connector, '_run_fetch', return_value=FAKE_SPREADSHEET) + mocker.patch.object(GoogleSheets2Connector, '_run_fetch', return_value=FAKE_SHEET) with pytest.raises(Exception) as err: con.get_df(ds) @@ -135,16 +144,15 @@ def test_spreadsheet_no_secrets(mocker, con, ds): con.get_df(ds) -def test_set_columns(mocker, con, ds): +def test_set_columns(mocker, con_with_secrets, ds): """It should return a well-formed column set.""" - con.set_secrets({'access_token': 'foo', 'refresh_token': 'bar'}) fake_results = { 'metadata': '...', 'values': [['Animateur', '', '', 'Week'], ['pika', '', 'a', 'W1'], ['bulbi', '', '', 'W2']], } mocker.patch.object(GoogleSheets2Connector, '_run_fetch', return_value=fake_results) - df = con.get_df(ds) + df = con_with_secrets.get_df(ds) assert df.to_dict() == { 'Animateur': {1: 'pika', 2: 'bulbi'}, 1: {1: '', 2: ''}, @@ -153,13 +161,43 @@ def test_set_columns(mocker, con, ds): } -@pytest.mark.skip(reason='Update seems to have broken this test') def test__run_fetch(mocker, con): """It should return a result from loops if all is ok.""" mocker.patch.object( - GoogleSheets2Connector, '_get_data', return_value=helpers.build_future(FAKE_SPREADSHEET) + GoogleSheets2Connector, '_get_data', return_value=helpers.build_future(FAKE_SHEET) ) result = con._run_fetch('/fudge', 'myaccesstoken') - assert result == FAKE_SPREADSHEET + assert result == FAKE_SHEET + + +def test_spreadsheet_without_sheet(mocker, con_with_secrets, ds_without_sheet): + """ + It should retrieve the first sheet of the spreadsheet if no sheet has been indicated + """ + + def mock_api_responses(uri: str, _token): + print('HERE', uri) + if uri.endswith('/Foo'): + return FAKE_SHEET + else: + return FAKE_SHEET_LIST_RESPONSE + + fetch_mock: Mock = mocker.patch.object( + GoogleSheets2Connector, '_run_fetch', side_effect=mock_api_responses + ) + df = con_with_secrets.get_df(ds_without_sheet) + + assert fetch_mock.call_count == 2 + assert ( + fetch_mock.call_args_list[0][0][0] + == 'https://sheets.googleapis.com/v4/spreadsheets/1SMnhnmBm-Tup3SfhS03McCf6S4pS2xqjI6CAXSSBpHU' + ) + assert ( + fetch_mock.call_args_list[1][0][0] + == 'https://sheets.googleapis.com/v4/spreadsheets/1SMnhnmBm-Tup3SfhS03McCf6S4pS2xqjI6CAXSSBpHU/values/Foo' + ) + + assert df.shape == (2, 2) + assert df.columns.tolist() == ['country', 'city'] diff --git a/toucan_connectors/google_sheets_2/google_sheets_2_connector.py b/toucan_connectors/google_sheets_2/google_sheets_2_connector.py index b2efcaa2f..d182cfec2 100644 --- a/toucan_connectors/google_sheets_2/google_sheets_2_connector.py +++ b/toucan_connectors/google_sheets_2/google_sheets_2_connector.py @@ -96,7 +96,7 @@ def _retrieve_data(self, data_source: GoogleSheets2DataSource) -> pd.DataFrame: if data_source.sheet is None: # Get spreadsheet informations and retrieve all the available sheets # https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/get - data = self._run_fetch(self.baseroute, access_token) + data = self._run_fetch(f'{self.baseroute}{data_source.spreadsheet_id}', access_token) available_sheets = [str(x['properties']['title']) for x in data['sheets']] data_source.sheet = available_sheets[0] From 94a4ceea30b3f26c5f6f360f561a5e8103aa28e5 Mon Sep 17 00:00:00 2001 From: David Nowinsky Date: Fri, 4 Sep 2020 12:46:28 +0200 Subject: [PATCH 11/13] test(gsheets): factor & document pydantic schema differences --- tests/google_sheets_2/test_google_sheets_2.py | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/tests/google_sheets_2/test_google_sheets_2.py b/tests/google_sheets_2/test_google_sheets_2.py index acea6a93c..99e36ee77 100644 --- a/tests/google_sheets_2/test_google_sheets_2.py +++ b/tests/google_sheets_2/test_google_sheets_2.py @@ -73,6 +73,17 @@ async def test_get_data(mocker, con): } +def get_columns_in_schema(schema): + """Pydantic generates schema slightly differently in python <=3.7 and in python 3.8""" + try: + if schema.get('definitions'): + return schema['definitions']['sheet']['enum'] + else: + return schema['properties']['sheet']['enum'] + except KeyError: + return None + + def test_get_form_with_secrets(mocker, con_with_secrets, ds): """It should return a list of spreadsheet titles.""" mocker.patch.object(GoogleSheets2Connector, '_run_fetch', return_value=FAKE_SHEET_LIST_RESPONSE) @@ -82,10 +93,7 @@ def test_get_form_with_secrets(mocker, con_with_secrets, ds): current_config={'spreadsheet_id': '1SMnhnmBm-Tup3SfhS03McCf6S4pS2xqjI6CAXSSBpHU'}, ) expected_results = ['Foo', 'Bar', 'Baz'] - if result.get('definitions'): - assert result['definitions']['sheet']['enum'] == expected_results - else: - assert result['properties']['sheet']['enum'] == expected_results + assert get_columns_in_schema(result) == expected_results def test_get_form_no_secrets(mocker, con, ds): @@ -95,10 +103,7 @@ def test_get_form_no_secrets(mocker, con, ds): connector=con, current_config={'spreadsheet_id': '1SMnhnmBm-Tup3SfhS03McCf6S4pS2xqjI6CAXSSBpHU'}, ) - if result.get('properties'): - assert not result['properties']['sheet'].get('enum') - else: - assert not result.get('definitions') + assert not get_columns_in_schema(result) def test_set_secrets(mocker, con): From f300b3b34cbfb5173b3be32031026c7b282914b1 Mon Sep 17 00:00:00 2001 From: David Nowinsky Date: Fri, 4 Sep 2020 12:48:11 +0200 Subject: [PATCH 12/13] doc(gsheet2): fix sheet param name --- doc/connectors/google_sheets_2.md | 2 +- tests/google_sheets_2/test_google_sheets_2.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/doc/connectors/google_sheets_2.md b/doc/connectors/google_sheets_2.md index 77bd70ad7..52c00a232 100644 --- a/doc/connectors/google_sheets_2.md +++ b/doc/connectors/google_sheets_2.md @@ -39,7 +39,7 @@ DATA_SOURCES: [ domain: '' name: '' spreadsheet_id: '' - sheetname: '' + sheet: '' skip_rows: , ... diff --git a/tests/google_sheets_2/test_google_sheets_2.py b/tests/google_sheets_2/test_google_sheets_2.py index 99e36ee77..271328402 100644 --- a/tests/google_sheets_2/test_google_sheets_2.py +++ b/tests/google_sheets_2/test_google_sheets_2.py @@ -183,7 +183,6 @@ def test_spreadsheet_without_sheet(mocker, con_with_secrets, ds_without_sheet): """ def mock_api_responses(uri: str, _token): - print('HERE', uri) if uri.endswith('/Foo'): return FAKE_SHEET else: From 14347b8d10b6c6822e1e779de0c86a6d1c95eef1 Mon Sep 17 00:00:00 2001 From: David Nowinsky Date: Fri, 4 Sep 2020 14:52:22 +0200 Subject: [PATCH 13/13] chore: fix typing for secrets --- tests/google_sheets_2/test_google_sheets_2.py | 6 +++--- .../google_sheets_2/google_sheets_2_connector.py | 7 +++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/google_sheets_2/test_google_sheets_2.py b/tests/google_sheets_2/test_google_sheets_2.py index 271328402..8982dda5b 100644 --- a/tests/google_sheets_2/test_google_sheets_2.py +++ b/tests/google_sheets_2/test_google_sheets_2.py @@ -19,7 +19,7 @@ def con(): @fixture def con_with_secrets(con): - con.set_secrets({'access_token': 'foo', 'refresh_token': 'bar'}) + con.set_secrets({'access_token': 'foo', 'refresh_token': None}) return con @@ -111,7 +111,7 @@ def test_set_secrets(mocker, con): spy = mocker.spy(GoogleSheets2Connector, 'set_secrets') fake_secrets = { 'access_token': 'myaccesstoken', - 'refresh_token': 'myrefreshtoken', + 'refresh_token': None, } con.set_secrets(fake_secrets) @@ -143,7 +143,7 @@ def test_spreadsheet_no_secrets(mocker, con, ds): assert str(err.value) == 'No credentials' - con.set_secrets({'refresh_token': 'myrefreshtoken'}) + con.set_secrets({'refresh_token': None}) with pytest.raises(KeyError): con.get_df(ds) diff --git a/toucan_connectors/google_sheets_2/google_sheets_2_connector.py b/toucan_connectors/google_sheets_2/google_sheets_2_connector.py index d182cfec2..dc6517c96 100644 --- a/toucan_connectors/google_sheets_2/google_sheets_2_connector.py +++ b/toucan_connectors/google_sheets_2/google_sheets_2_connector.py @@ -51,6 +51,9 @@ def get_form(cls, connector: 'GoogleSheets2Connector', current_config): return create_model('FormSchema', **constraints, __base__=cls).schema() +Secrets = Dict[str, Any] + + class GoogleSheets2Connector(ToucanConnector): """The Google Sheets connector.""" @@ -62,7 +65,7 @@ class GoogleSheets2Connector(ToucanConnector): baseroute = 'https://sheets.googleapis.com/v4/spreadsheets/' - secrets: Optional[Dict[str, Any]] + secrets: Optional[Secrets] async def _get_data(self, url, access_token): """Build the final request along with headers.""" @@ -71,7 +74,7 @@ async def _get_data(self, url, access_token): async with ClientSession(headers=headers) as session: return await fetch(url, session) - def set_secrets(self, secrets: Dict[str, str]): + def set_secrets(self, secrets: Secrets): """Set the secrets from inside the main service.""" self.secrets = secrets