From 4a39dee66a5a305e48e42cfad7dfafdcda799876 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Thu, 10 Nov 2016 21:27:38 -0800 Subject: [PATCH 1/5] Changing base URL for translate API. --- translate/google/cloud/translate/connection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/translate/google/cloud/translate/connection.py b/translate/google/cloud/translate/connection.py index 453a612edfa9..608bb48d913e 100644 --- a/translate/google/cloud/translate/connection.py +++ b/translate/google/cloud/translate/connection.py @@ -20,7 +20,7 @@ class Connection(_http.JSONConnection): """A connection to Google Cloud Translate via the JSON REST API.""" - API_BASE_URL = 'https://www.googleapis.com' + API_BASE_URL = 'https://translation.googleapis.com' """The base of the API call URL.""" API_VERSION = 'v2' From 3e707e55454d20867e1d15a83416c946ac223a3b Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Thu, 10 Nov 2016 21:56:56 -0800 Subject: [PATCH 2/5] Adding credentials support to Translate client. As a side effect, also removing the need for the GOOGLE_CLOUD_TESTS_API_KEY in system tests, so removing all references to it in the codebase. --- CONTRIBUTING.rst | 2 - system_tests/local_test_setup.sample | 1 - system_tests/translate.py | 8 +--- translate/google/cloud/translate/client.py | 45 ++++++++++++------- .../google/cloud/translate/connection.py | 3 ++ translate/unit_tests/test_client.py | 41 +++++++++-------- 6 files changed, 55 insertions(+), 45 deletions(-) diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index ebf34a43cb75..e7f5bf193753 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -235,8 +235,6 @@ Running System Tests `docs `__ for more details. In order for Logging system tests to work, the Service Account will also have to be made a project Owner. This can be changed under "IAM & Admin". - - ``GOOGLE_CLOUD_TESTS_API_KEY``: The API key for your project with - the Google Translate API (and others) enabled. - Examples of these can be found in ``system_tests/local_test_setup.sample``. We recommend copying this to ``system_tests/local_test_setup``, editing the diff --git a/system_tests/local_test_setup.sample b/system_tests/local_test_setup.sample index 4401bc4e72e4..63eb733a7c85 100644 --- a/system_tests/local_test_setup.sample +++ b/system_tests/local_test_setup.sample @@ -1,4 +1,3 @@ export GOOGLE_APPLICATION_CREDENTIALS="app_credentials.json.sample" export GOOGLE_CLOUD_TESTING_REMOTE="upstream" export GOOGLE_CLOUD_TESTING_BRANCH="master" -export GOOGLE_CLOUD_TESTS_API_KEY="abcd1234" diff --git a/system_tests/translate.py b/system_tests/translate.py index ce016fbf77b4..b02dba9a949e 100644 --- a/system_tests/translate.py +++ b/system_tests/translate.py @@ -13,16 +13,11 @@ # limitations under the License. -import os - import unittest from google.cloud import translate -ENV_VAR = 'GOOGLE_CLOUD_TESTS_API_KEY' - - class Config(object): """Run-time configuration to be modified at set-up. @@ -33,8 +28,7 @@ class Config(object): def setUpModule(): - api_key = os.getenv(ENV_VAR) - Config.CLIENT = translate.Client(api_key=api_key) + Config.CLIENT = translate.Client() class TestTranslate(unittest.TestCase): diff --git a/translate/google/cloud/translate/client.py b/translate/google/cloud/translate/client.py index f28bd996dffd..31aff3834db5 100644 --- a/translate/google/cloud/translate/client.py +++ b/translate/google/cloud/translate/client.py @@ -15,10 +15,10 @@ """Client for interacting with the Google Cloud Translate API.""" -import httplib2 import six from google.cloud._helpers import _to_bytes +from google.cloud.client import Client as BaseClient from google.cloud.translate.connection import Connection @@ -26,29 +26,36 @@ """ISO 639-1 language code for English.""" -class Client(object): +class Client(BaseClient): """Client to bundle configuration needed for API requests. + :type target_language: str + :param target_language: (Optional) The target language used for + translations and language names. (Defaults to + :data:`ENGLISH_ISO_639`.) + :type api_key: str - :param api_key: The key used to send with requests as a query - parameter. + :param api_key: (Optional) The key used to send with requests as a + query parameter. + + :type credentials: :class:`oauth2client.client.OAuth2Credentials` + :param credentials: (Optional) The OAuth2 Credentials to use for the + connection owned by this client. If not passed (and + if no ``http`` object is passed), falls back to the + default inferred from the environment. :type http: :class:`httplib2.Http` or class that defines ``request()``. :param http: (Optional) HTTP object to make requests. If not passed, an :class:`httplib.Http` object is created. - - :type target_language: str - :param target_language: (Optional) The target language used for - translations and language names. (Defaults to - :data:`ENGLISH_ISO_639`.) """ - def __init__(self, api_key, http=None, target_language=ENGLISH_ISO_639): + _connection_class = Connection + + def __init__(self, target_language=ENGLISH_ISO_639, api_key=None, + credentials=None, http=None): self.api_key = api_key - if http is None: - http = httplib2.Http() - self._connection = Connection(http=http) self.target_language = target_language + super(Client, self).__init__(credentials=credentials, http=http) def get_languages(self, target_language=None): """Get list of supported languages for translation. @@ -70,7 +77,9 @@ def get_languages(self, target_language=None): dictionary will also contain the name of each supported language (localized to the target language). """ - query_params = {'key': self.api_key} + query_params = {} + if self.api_key is not None: + query_params['key'] = self.api_key if target_language is None: target_language = self.target_language if target_language is not None: @@ -114,7 +123,9 @@ def detect_language(self, values): single_value = True values = [values] - query_params = [('key', self.api_key)] + query_params = [] + if self.api_key is not None: + query_params.append(('key', self.api_key)) query_params.extend(('q', _to_bytes(value, 'utf-8')) for value in values) response = self._connection.api_request( @@ -199,7 +210,9 @@ def translate(self, values, target_language=None, format_=None, if isinstance(customization_ids, six.string_types): customization_ids = [customization_ids] - query_params = [('key', self.api_key), ('target', target_language)] + query_params = [('target', target_language)] + if self.api_key is not None: + query_params.append(('key', self.api_key)) query_params.extend(('q', _to_bytes(value, 'utf-8')) for value in values) query_params.extend(('cid', cid) for cid in customization_ids) diff --git a/translate/google/cloud/translate/connection.py b/translate/google/cloud/translate/connection.py index 608bb48d913e..0582bcd22a3f 100644 --- a/translate/google/cloud/translate/connection.py +++ b/translate/google/cloud/translate/connection.py @@ -28,3 +28,6 @@ class Connection(_http.JSONConnection): API_URL_TEMPLATE = '{api_base_url}/language/translate/{api_version}{path}' """A template for the URL of a particular API call.""" + + SCOPE = ('https://www.googleapis.com/auth/cloud-platform',) + """The scopes required for authenticating.""" diff --git a/translate/unit_tests/test_client.py b/translate/unit_tests/test_client.py index f3a9ffb76871..8338f7f70855 100644 --- a/translate/unit_tests/test_client.py +++ b/translate/unit_tests/test_client.py @@ -32,10 +32,11 @@ def test_ctor(self): from google.cloud.translate.client import ENGLISH_ISO_639 http = object() - client = self._make_one(self.KEY, http=http) + client = self._make_one(http=http) self.assertIsInstance(client._connection, Connection) self.assertIsNone(client._connection.credentials) self.assertIs(client._connection.http, http) + self.assertIsNone(client.api_key) self.assertEqual(client.target_language, ENGLISH_ISO_639) def test_ctor_non_default(self): @@ -43,16 +44,18 @@ def test_ctor_non_default(self): http = object() target = 'es' - client = self._make_one(self.KEY, http=http, target_language=target) + client = self._make_one( + target_language=target, api_key=self.KEY, http=http) self.assertIsInstance(client._connection, Connection) self.assertIsNone(client._connection.credentials) self.assertIs(client._connection.http, http) + self.assertEqual(self.KEY, client.api_key) self.assertEqual(client.target_language, target) def test_get_languages(self): from google.cloud.translate.client import ENGLISH_ISO_639 - client = self._make_one(self.KEY) + client = self._make_one(api_key=self.KEY, http=object()) supported = [ {'language': 'en', 'name': 'English'}, {'language': 'af', 'name': 'Afrikaans'}, @@ -77,7 +80,8 @@ def test_get_languages(self): {'key': self.KEY, 'target': ENGLISH_ISO_639}) def test_get_languages_no_target(self): - client = self._make_one(self.KEY, target_language=None) + client = self._make_one( + target_language=None, http=object()) supported = [ {'language': 'en'}, {'language': 'af'}, @@ -96,12 +100,13 @@ def test_get_languages_no_target(self): # Verify requested. self.assertEqual(len(conn._requested), 1) req = conn._requested[0] + self.assertEqual(len(req), 3) self.assertEqual(req['method'], 'GET') self.assertEqual(req['path'], '/languages') - self.assertEqual(req['query_params'], {'key': self.KEY}) + self.assertEqual(req['query_params'], {}) def test_get_languages_explicit_target(self): - client = self._make_one(self.KEY) + client = self._make_one(api_key=self.KEY, http=object()) target_language = 'en' supported = [ {'language': 'en', 'name': 'Spanish'}, @@ -127,7 +132,7 @@ def test_get_languages_explicit_target(self): {'key': self.KEY, 'target': target_language}) def test_detect_language_bad_result(self): - client = self._make_one(self.KEY) + client = self._make_one(api_key=self.KEY, http=object()) value = 'takoy' conn = client._connection = _Connection({}) @@ -146,7 +151,7 @@ def test_detect_language_bad_result(self): self.assertEqual(req['query_params'], query_params) def test_detect_language_single_value(self): - client = self._make_one(self.KEY) + client = self._make_one(api_key=self.KEY, http=object()) value = 'takoy' detection = { 'confidence': 1.0, @@ -176,7 +181,7 @@ def test_detect_language_single_value(self): self.assertEqual(req['query_params'], query_params) def test_detect_language_multiple_values(self): - client = self._make_one(self.KEY) + client = self._make_one(http=object()) value1 = u'fa\xe7ade' # facade (with a cedilla) detection1 = { 'confidence': 0.6166008, @@ -210,14 +215,13 @@ def test_detect_language_multiple_values(self): self.assertEqual(req['method'], 'GET') self.assertEqual(req['path'], '/detect') query_params = [ - ('key', self.KEY), ('q', value1.encode('utf-8')), ('q', value2.encode('utf-8')), ] self.assertEqual(req['query_params'], query_params) def test_detect_language_multiple_results(self): - client = self._make_one(self.KEY) + client = self._make_one(api_key=self.KEY, http=object()) value = 'soy' detection1 = { 'confidence': 0.81496066, @@ -242,7 +246,7 @@ def test_detect_language_multiple_results(self): client.detect_language(value) def test_translate_bad_result(self): - client = self._make_one(self.KEY) + client = self._make_one(api_key=self.KEY, http=object()) value = 'hvala ti' conn = client._connection = _Connection({}) @@ -255,14 +259,14 @@ def test_translate_bad_result(self): self.assertEqual(req['method'], 'GET') self.assertEqual(req['path'], '') query_params = [ - ('key', self.KEY), ('target', 'en'), + ('key', self.KEY), ('q', value.encode('utf-8')), ] self.assertEqual(req['query_params'], query_params) def test_translate_defaults(self): - client = self._make_one(self.KEY) + client = self._make_one(http=object()) value = 'hvala ti' translation = { 'detectedSourceLanguage': 'hr', @@ -285,14 +289,13 @@ def test_translate_defaults(self): self.assertEqual(req['method'], 'GET') self.assertEqual(req['path'], '') query_params = [ - ('key', self.KEY), ('target', 'en'), ('q', value.encode('utf-8')), ] self.assertEqual(req['query_params'], query_params) def test_translate_multiple(self): - client = self._make_one(self.KEY) + client = self._make_one(api_key=self.KEY, http=object()) value1 = 'hvala ti' translation1 = { 'detectedSourceLanguage': 'hr', @@ -321,15 +324,15 @@ def test_translate_multiple(self): self.assertEqual(req['method'], 'GET') self.assertEqual(req['path'], '') query_params = [ - ('key', self.KEY), ('target', 'en'), + ('key', self.KEY), ('q', value1.encode('utf-8')), ('q', value2.encode('utf-8')), ] self.assertEqual(req['query_params'], query_params) def test_translate_explicit(self): - client = self._make_one(self.KEY) + client = self._make_one(api_key=self.KEY, http=object()) value = 'thank you' target_language = 'eo' source_language = 'en' @@ -357,8 +360,8 @@ def test_translate_explicit(self): self.assertEqual(req['method'], 'GET') self.assertEqual(req['path'], '') query_params = [ - ('key', self.KEY), ('target', target_language), + ('key', self.KEY), ('q', value.encode('utf-8')), ('cid', cid), ('format', format_), From 1f40989d1d6cca7a173e77e7289e2918bd72291e Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Thu, 10 Nov 2016 22:32:44 -0800 Subject: [PATCH 3/5] Adding support for model parameter in translate API. --- system_tests/translate.py | 4 ++-- translate/google/cloud/translate/client.py | 14 +++++++++++--- translate/unit_tests/test_client.py | 5 ++++- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/system_tests/translate.py b/system_tests/translate.py index b02dba9a949e..6ef78b0f0b95 100644 --- a/system_tests/translate.py +++ b/system_tests/translate.py @@ -55,8 +55,8 @@ def test_detect_language(self): def test_translate(self): values = ['hvala ti', 'dankon', 'Me llamo Jeff', 'My name is Jeff'] - translations = Config.CLIENT.translate(values, - target_language='de') + translations = Config.CLIENT.translate( + values, target_language='de', model='nmt') self.assertEqual(len(values), len(translations)) self.assertEqual( diff --git a/translate/google/cloud/translate/client.py b/translate/google/cloud/translate/client.py index 31aff3834db5..c43acb80550b 100644 --- a/translate/google/cloud/translate/client.py +++ b/translate/google/cloud/translate/client.py @@ -157,7 +157,8 @@ def detect_language(self, values): return detections def translate(self, values, target_language=None, format_=None, - source_language=None, customization_ids=()): + source_language=None, customization_ids=(), + model=None): """Translate a string or list of strings. See: https://cloud.google.com/translate/v2/\ @@ -184,7 +185,11 @@ def translate(self, values, target_language=None, format_=None, for translation. Sets the ``cid`` parameter in the query. - :rtype: str or list list + :type model: str + :param model: (Optional) The model used to translate the text. The + only accepted values are ``base`` and ``nmt``. + + :rtype: str or list :returns: A list of dictionaries for each queried value. Each dictionary typically contains three keys (though not all will be present in all cases) @@ -194,10 +199,11 @@ def translate(self, values, target_language=None, format_=None, * ``translatedText``: The translation of the text into the target language. * ``input``: The corresponding input value. + * ``model``: The model used to translate the text. If only a single value is passed, then only a single dictionary will be returned. - :raises: :class:`ValueError ` if the number of + :raises: :class:`~exceptions.ValueError` if the number of values and translations differ. """ single_value = False @@ -220,6 +226,8 @@ def translate(self, values, target_language=None, format_=None, query_params.append(('format', format_)) if source_language is not None: query_params.append(('source', source_language)) + if model is not None: + query_params.append(('model', model)) response = self._connection.api_request( method='GET', path='', query_params=query_params) diff --git a/translate/unit_tests/test_client.py b/translate/unit_tests/test_client.py index 8338f7f70855..0fa04897af5f 100644 --- a/translate/unit_tests/test_client.py +++ b/translate/unit_tests/test_client.py @@ -349,9 +349,11 @@ def test_translate_explicit(self): cid = '123' format_ = 'text' + model = 'nmt' result = client.translate(value, target_language=target_language, source_language=source_language, - format_=format_, customization_ids=cid) + format_=format_, customization_ids=cid, + model=model) self.assertEqual(result, translation) # Verify requested. @@ -366,6 +368,7 @@ def test_translate_explicit(self): ('cid', cid), ('format', format_), ('source', source_language), + ('model', model), ] self.assertEqual(req['query_params'], query_params) From 735c8d028dc104ef69de1fd87258cc4ea2d3f20f Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 14 Nov 2016 11:28:34 -0800 Subject: [PATCH 4/5] Disabling default credentials if translate used with API key. --- translate/google/cloud/translate/client.py | 7 +++++++ translate/tox.ini | 1 + translate/unit_tests/test_client.py | 20 ++++++++++++++++++-- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/translate/google/cloud/translate/client.py b/translate/google/cloud/translate/client.py index c43acb80550b..b7fe9142d017 100644 --- a/translate/google/cloud/translate/client.py +++ b/translate/google/cloud/translate/client.py @@ -15,6 +15,7 @@ """Client for interacting with the Google Cloud Translate API.""" +import httplib2 import six from google.cloud._helpers import _to_bytes @@ -55,6 +56,12 @@ def __init__(self, target_language=ENGLISH_ISO_639, api_key=None, credentials=None, http=None): self.api_key = api_key self.target_language = target_language + + if api_key is not None: + # If API key auth is desired, make it so that no credentials + # will be auto-detected by the base class constructor. + if http is None: + http = httplib2.Http() super(Client, self).__init__(credentials=credentials, http=http) def get_languages(self, target_language=None): diff --git a/translate/tox.ini b/translate/tox.ini index f33c168d61c2..168559383e71 100644 --- a/translate/tox.ini +++ b/translate/tox.ini @@ -6,6 +6,7 @@ envlist = localdeps = pip install --quiet --upgrade {toxinidir}/../core deps = + mock pytest covercmd = py.test --quiet \ diff --git a/translate/unit_tests/test_client.py b/translate/unit_tests/test_client.py index 0fa04897af5f..2e9a23d6995e 100644 --- a/translate/unit_tests/test_client.py +++ b/translate/unit_tests/test_client.py @@ -27,7 +27,7 @@ def _get_target_class(): def _make_one(self, *args, **kw): return self._get_target_class()(*args, **kw) - def test_ctor(self): + def test_constructor(self): from google.cloud.translate.connection import Connection from google.cloud.translate.client import ENGLISH_ISO_639 @@ -39,7 +39,7 @@ def test_ctor(self): self.assertIsNone(client.api_key) self.assertEqual(client.target_language, ENGLISH_ISO_639) - def test_ctor_non_default(self): + def test_constructor_non_default(self): from google.cloud.translate.connection import Connection http = object() @@ -52,6 +52,22 @@ def test_ctor_non_default(self): self.assertEqual(self.KEY, client.api_key) self.assertEqual(client.target_language, target) + def test_constructor_api_key_override(self): + import mock + from google.cloud.translate.connection import Connection + + target = 'ru' + with mock.patch('httplib2.Http') as http_ctor: + client = self._make_one( + target_language=target, api_key=self.KEY) + + http_ctor.assert_called_once_with() + self.assertIsInstance(client._connection, Connection) + self.assertIsNone(client._connection.credentials) + self.assertIs(client._connection.http, http_ctor.return_value) + self.assertEqual(self.KEY, client.api_key) + self.assertEqual(client.target_language, target) + def test_get_languages(self): from google.cloud.translate.client import ENGLISH_ISO_639 From 2da002058bfa2bdb31bb902ac75393d1f2410ae4 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 14 Nov 2016 11:37:19 -0800 Subject: [PATCH 5/5] Adding BASE and NMT constants for translation. --- system_tests/translate.py | 2 +- translate/google/cloud/translate/__init__.py | 2 ++ translate/google/cloud/translate/client.py | 8 +++++++- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/system_tests/translate.py b/system_tests/translate.py index 6ef78b0f0b95..402be16399be 100644 --- a/system_tests/translate.py +++ b/system_tests/translate.py @@ -56,7 +56,7 @@ def test_translate(self): values = ['hvala ti', 'dankon', 'Me llamo Jeff', 'My name is Jeff'] translations = Config.CLIENT.translate( - values, target_language='de', model='nmt') + values, target_language='de', model=translate.NMT) self.assertEqual(len(values), len(translations)) self.assertEqual( diff --git a/translate/google/cloud/translate/__init__.py b/translate/google/cloud/translate/__init__.py index c58b22301ee5..83ff5f114435 100644 --- a/translate/google/cloud/translate/__init__.py +++ b/translate/google/cloud/translate/__init__.py @@ -14,5 +14,7 @@ """Google Cloud Translate API wrapper.""" +from google.cloud.translate.client import BASE from google.cloud.translate.client import Client +from google.cloud.translate.client import NMT from google.cloud.translate.connection import Connection diff --git a/translate/google/cloud/translate/client.py b/translate/google/cloud/translate/client.py index b7fe9142d017..47c692833abf 100644 --- a/translate/google/cloud/translate/client.py +++ b/translate/google/cloud/translate/client.py @@ -26,6 +26,12 @@ ENGLISH_ISO_639 = 'en' """ISO 639-1 language code for English.""" +BASE = 'base' +"""Base translation model.""" + +NMT = 'nmt' +"""Neural Machine Translation model.""" + class Client(BaseClient): """Client to bundle configuration needed for API requests. @@ -194,7 +200,7 @@ def translate(self, values, target_language=None, format_=None, :type model: str :param model: (Optional) The model used to translate the text. The - only accepted values are ``base`` and ``nmt``. + only accepted values are :attr:`BASE` and :attr:`NMT`. :rtype: str or list :returns: A list of dictionaries for each queried value. Each