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

Update Translate API Client #240

Merged
merged 5 commits into from
Nov 14, 2016
Merged

Update Translate API Client #240

merged 5 commits into from
Nov 14, 2016

Conversation

jdpedrie
Copy link
Contributor

@jdpedrie jdpedrie commented Nov 10, 2016

Please hold this until 11/15.

  • Support service account and application default credentials in Translate
  • Add support for the new model field.

Note that this change does not switch the operation from GET to POST.

* Support service account and application default credentials in
  Translate
* Add support for the new `model` field.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 10, 2016
@jdpedrie jdpedrie added do not merge Indicates a pull request not ready for merge, due to either quality or timing. api: translation Issues related to the Cloud Translation API. labels Nov 10, 2016
@michaelbausor
Copy link
Contributor

LGTM, let others take a look also.

@@ -50,7 +56,8 @@ public function testTranslate()
$options = [
'source' => $expected['source'],
'target' => 'de',
'format' => 'text'
'format' => 'text',
'model' => 'base'

This comment was marked as spam.

@@ -134,6 +134,8 @@ public function __construct(array $config = [])
* @type string $format Indicates whether the string to be translated is
* either plain-text or HTML. Acceptable values are `html` or
* `text`. **Defaults to** `"html"`.
* @type string $model The model to use for the translation request. May
* be `nmt` or `base`. **Defaults to** an empty string.
* }
* @return array A translation result including a `source` key containing

This comment was marked as spam.

Copy link
Contributor

@dwsupplee dwsupplee left a comment

Choose a reason for hiding this comment

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

LGTM besides otherwise noted

* Please note that unlike most other Cloud Platform services Google Translate
* requires a public API access key and cannot currently be accessed with a
* service account or application default credentials. Follow the
* Please note that while Google Translate supports authentication via service

This comment was marked as spam.

@@ -92,20 +90,22 @@ class TranslateClient
*/
public function __construct(array $config = [])

This comment was marked as spam.

$response = $this->connection->listTranslations($options + [
'q' => $strings,
'key' => $this->key,
'target' => $this->targetLanguage
'target' => $this->targetLanguage,
'model' => $options['model']

This comment was marked as spam.

'source' => $source,
'input' => $strings[$key],
'text' => $translation['translatedText'],
'model' => (isset($translation['model']))

This comment was marked as spam.

@jdpedrie
Copy link
Contributor Author

Updated! PTAL @tmatsuo @dwsupplee @akeran

@jdpedrie jdpedrie merged commit f198981 into googleapis:master Nov 14, 2016
@jdpedrie jdpedrie deleted the translate-update branch November 14, 2016 21:56
@jdpedrie jdpedrie mentioned this pull request Nov 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: translation Issues related to the Cloud Translation API. cla: yes This human has signed the Contributor License Agreement. do not merge Indicates a pull request not ready for merge, due to either quality or timing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants