Skip to content

Basic implementation of Translate API. #1772

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
May 24, 2016
Merged

Basic implementation of Translate API. #1772

merged 11 commits into from
May 24, 2016

Conversation

@dhermes
Copy link
Contributor

@dhermes dhermes commented May 3, 2016 •
edited
Loading

Test forthcoming after API discussion.

Tests added in 2nd commit.

/cc @daspecster

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 3, 2016
@dhermes dhermes added the api: translation Issues related to the Cloud Translation API API. label May 3, 2016
@dhermes
Copy link
Contributor Author

dhermes commented May 3, 2016

I just added a GCLOUD_TESTS_API_KEY environment variable to Travis with an API key for our project tied to Travis.

daspecster
daspecster reviewed May 3, 2016
View reviewed changes
gcloud/translate/__init__.py Outdated
@@ -0,0 +1,18 @@
# Copyright 2015 Google Inc. All rights reserved.

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@dhermes dhermes force-pushed the start-translate-api branch from 6faad34 to 03b4442 Compare May 11, 2016 17:49
@tseaver
Copy link
Contributor

tseaver commented May 12, 2016

pylint failure:

gcloud/translate/client.py:108:80: E501 line too long (80 > 79 characters)

@dhermes dhermes force-pushed the start-translate-api branch from 3fd72e2 to 92d4e3e Compare May 13, 2016 00:07
@dhermes
Copy link
Contributor Author

dhermes commented May 13, 2016

D'oh! Fixed the failure. @tseaver PTAL.

@jgeewax WDYT of the verb and verb_multiple approach (as is done in datastore)?

@tseaver
Copy link
Contributor

tseaver commented May 13, 2016

LGTM

@jgeewax
Copy link
Contributor

jgeewax commented May 13, 2016

@dhermes : Didn't notice the updated comment -- lmk what you think.

tseaver
tseaver reviewed May 16, 2016
View reviewed changes
docs/translate-client.rst

.. automodule:: gcloud.translate.client
:members:
:undoc-members:

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

tseaver
tseaver reviewed May 16, 2016
View reviewed changes
gcloud/translate/client.py
query_params = {'key': self.key}
if target_language is None:
target_language = self.target_language
if target_language is not None:

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

dhermes added 7 commits May 17, 2016 09:10
@dhermes
Basic implementation of Translate API.
6e566c6 
Test forthcoming after API discussion.
@dhermes
Adding full test coverage.
039ae5e 
Also fixing a few broken docs.
@dhermes
Adding system tests for translate API.
8a3a97d
@dhermes
Addressing review feedback.
01e8f9a 
- Dropping _zip_assert
- Making multiple detections for a single value disallowed
- Allow a default target language
@dhermes
Fix Py3k failures.
13fb593
@dhermes
Fixing lint errors.
92e92a2
@dhermes
Reverting back to using str/list values in Translate.
2a4eaf8
@dhermes dhermes force-pushed the start-translate-api branch from 92d4e3e to 2a4eaf8 Compare May 17, 2016 16:33
@dhermes
Copy link
Contributor Author

dhermes commented May 17, 2016

@tseaver @jgeewax PTAL. I reverted back to the original API surface.

jgeewax
jgeewax reviewed May 17, 2016
View reviewed changes
gcloud/translate/client.py Outdated
class Client(object):
"""Client to bundle configuration needed for API requests.

:type key: str

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@tseaver
Copy link
Contributor

tseaver commented May 17, 2016

@dhermes pylint failures: https://travis-ci.org/GoogleCloudPlatform/gcloud-python/builds/130895364#L481-L482

@dhermes
Copy link
Contributor Author

dhermes commented May 17, 2016

Thanks for the Pylint pointer. It was worse than a lint failure, was actually a broken system test (using the old key instead of api_key in the Client ctor).

@dhermes dhermes mentioned this pull request May 17, 2016
Closed
6 tasks
@dhermes
Copy link
Contributor Author

dhermes commented May 23, 2016

@tseaver @jgeewax Where do we stand here?

@tseaver
Copy link
Contributor

tseaver commented May 23, 2016

LGTM

@dhermes
Copy link
Contributor Author

dhermes commented May 23, 2016

@jgeewax I'll merge in 24 hours if I don't hear from you.

@jgeewax
Copy link
Contributor

jgeewax commented May 24, 2016

lgtm

@dhermes dhermes merged commit 503cd32 into googleapis:master May 24, 2016
@dhermes dhermes deleted the start-translate-api branch May 24, 2016 18:11
@dhermes
Copy link
Contributor Author

dhermes commented May 24, 2016

Blerg! I forgot to squash commits. I wish GitHub had a way to block until squashed (if the author so chooses).

@dhermes dhermes mentioned this pull request Jun 7, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@tseaver tseaver

Labels

api: translation Issues related to the Cloud Translation API API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@dhermes @tseaver @jgeewax @daspecster @googlebot