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

Google translate #1978

Merged
merged 12 commits into from
Apr 26, 2017
5 changes: 3 additions & 2 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,9 @@ gem 'hypdf', '~> 1.0.10' # PDFInfoAgent
# FIXME needs to loosen omniauth dependency
gem 'weibo_2', github: 'dsander/weibo_2', branch: 'master'

# GoogleCalendarPublishAgent
gem "google-api-client", require: 'google/api_client'
Copy link
Member

@cantino cantino Apr 25, 2017

Choose a reason for hiding this comment

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

Can you add back in the require: 'google/api_client' so that the GoogleCalendarPublishAgent will also work ok? I think you should also copy the gem_dependency_check and #{'## Include "google-api-client" in your Gemfile to use this Agent!' if dependencies_missing?} lines from GoogleCalendarPublishAgent into your Agent so that it correctly detects if google-api-client has been removed from the Gemfile, and remove the require line that you added at the top.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done and done!

# GoogleCalendarPublishAgent and GoogleTranslateAgent
gem 'google-api-client', '~> 0.9.20'
gem 'jwt', '~> 1.5'

# Twitter Agents
gem 'twitter', '~> 5.14.0' # Must to be loaded before cantino-twitter-stream.
Expand Down
62 changes: 37 additions & 25 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,6 @@ GEM
tzinfo (~> 1.1)
addressable (2.3.8)
arel (7.1.4)
autoparse (0.3.3)
addressable (>= 2.3.1)
extlib (>= 0.9.15)
multi_json (>= 1.0.0)
aws-sdk-core (2.2.15)
jmespath (~> 1.0)
bcrypt (3.1.11)
Expand Down Expand Up @@ -213,7 +209,6 @@ GEM
evernote-thrift
oauth (>= 0.4.1)
execjs (2.6.0)
extlib (0.9.16)
faraday (0.9.2)
multipart-post (>= 1.2, < 3)
feedjira (2.1.0)
Expand All @@ -239,17 +234,23 @@ GEM
rails (>= 3.0)
globalid (0.3.7)
activesupport (>= 4.1.0)
google-api-client (0.7.1)
addressable (>= 2.3.2)
autoparse (>= 0.3.3)
extlib (>= 0.9.15)
faraday (>= 0.9.0)
jwt (>= 0.1.5)
launchy (>= 2.1.1)
multi_json (>= 1.0.0)
retriable (>= 1.4)
signet (>= 0.5.0)
uuidtools (>= 2.1.0)
google-api-client (0.9.28)
Copy link

Choose a reason for hiding this comment

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

is there a reason to upgrade to google-api-client 0.9.28 from 0.7.1 ? because based on changelog the version 0.9 introduces breaking changes and we can check this guide for migration: link

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm pretty sure there was but I can't remember it 😛 I'll have a look again and see if it works with the old one. Also, the specs passed so I figured it didn't matter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although if the spec for calendar or whatever it was is stubbed I reckon that won't help much..

Copy link

Choose a reason for hiding this comment

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

I suggest to keep the same version if it works to avoid regression. :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree keeping the old version would be better, otherwise we need to manually verify the calendar agent is still working.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the main reason was because the old version seems to be a pain to use and all the examples I could find for translate was for a newer version. I'll see if I can revert tho.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You are right, it looks like 0.9.11 was the first version to add the translation API.

addressable (~> 2.3)
googleauth (~> 0.5)
httpclient (~> 2.7)
hurley (~> 0.1)
memoist (~> 0.11)
mime-types (>= 1.6)
representable (~> 2.3.0)
retriable (~> 2.0)
googleauth (0.5.1)
faraday (~> 0.9)
jwt (~> 1.4)
logging (~> 2.0)
memoist (~> 0.12)
multi_json (~> 1.11)
os (~> 0.9)
signet (~> 0.7)
guard (2.13.0)
formatador (>= 0.2.4)
listen (>= 2.7, <= 4.0)
Expand Down Expand Up @@ -285,8 +286,10 @@ GEM
httparty (0.13.7)
json (~> 1.8)
multi_xml (>= 0.5.2)
httpclient (2.8.3)
huginn_agent (0.4.0)
thor
hurley (0.2)
hypdf (1.0.10)
httmultiparty (~> 0.3)
httparty (~> 0.13)
Expand All @@ -299,7 +302,7 @@ GEM
json (1.8.3)
jsonpathv2 (0.0.8)
multi_json
jwt (1.4.1)
jwt (1.5.6)
kgio (2.10.0)
kramdown (1.3.3)
launchy (2.4.2)
Expand All @@ -315,13 +318,18 @@ GEM
listen (3.0.5)
rb-fsevent (>= 0.9.3)
rb-inotify (>= 0.9)
little-plugger (1.1.4)
logging (2.2.2)
little-plugger (~> 1.1)
multi_json (~> 1.10)
loofah (2.0.3)
nokogiri (>= 1.5.9)
lumberjack (1.0.10)
macaddr (1.7.1)
systemu (~> 2.6.2)
mail (2.6.4)
mime-types (>= 1.16, < 4)
memoist (0.15.0)
memoizable (0.4.2)
thread_safe (~> 0.3, >= 0.3.1)
method_source (0.8.2)
Expand Down Expand Up @@ -383,6 +391,7 @@ GEM
omniauth (~> 1.0)
omniauth-oauth2 (~> 1.1)
orm_adapter (0.5.0)
os (0.9.6)
pg (0.18.3)
poltergeist (1.8.1)
capybara (~> 2.1)
Expand Down Expand Up @@ -439,13 +448,15 @@ GEM
rb-kqueue (0.2.4)
ffi (>= 0.5.0)
ref (2.0.0)
representable (2.3.0)
uber (~> 0.0.7)
responders (2.3.0)
railties (>= 4.2.0, < 5.1)
rest-client (1.8.0)
http-cookie (>= 1.0.2, < 2.0)
mime-types (>= 1.16, < 3.0)
netrc (~> 0.7)
retriable (2.0.2)
retriable (2.1.0)
rr (1.1.2)
rspec (3.5.0)
rspec-core (~> 3.5.0)
Expand Down Expand Up @@ -495,11 +506,11 @@ GEM
shellany (0.0.1)
shoulda-matchers (3.0.0)
activesupport (>= 4.0.0)
signet (0.5.1)
addressable (>= 2.2.3)
faraday (>= 0.9.0.rc5)
jwt (>= 0.1.5)
multi_json (>= 1.0.0)
signet (0.7.3)
addressable (~> 2.3)
faraday (~> 0.9)
jwt (~> 1.5)
multi_json (~> 1.10)
simple_oauth (0.3.1)
simplecov (0.9.2)
docile (~> 1.1.0)
Expand Down Expand Up @@ -558,6 +569,7 @@ GEM
ethon (>= 0.7.1)
tzinfo (1.2.2)
thread_safe (~> 0.1)
uber (0.0.15)
uglifier (2.7.2)
execjs (>= 0.3.0)
json (>= 1.8.0)
Expand All @@ -569,7 +581,6 @@ GEM
raindrops (~> 0.7)
uuid (2.3.7)
macaddr (~> 1.0)
uuidtools (2.1.5)
vcr (2.9.2)
warden (1.2.6)
rack (>= 1.0)
Expand Down Expand Up @@ -628,7 +639,7 @@ DEPENDENCIES
foreman (~> 0.63.0)
geokit (~> 1.8.4)
geokit-rails (~> 2.2.0)
google-api-client
google-api-client (~> 0.9.20)
guard (~> 2.13.0)
guard-livereload (~> 2.5.1)
guard-rspec (~> 4.6.4)
Expand All @@ -641,6 +652,7 @@ DEPENDENCIES
jquery-rails (~> 4.2.1)
json (~> 1.8.1)
jsonpathv2 (~> 0.0.8)
jwt (~> 1.5)
kaminari!
kramdown (~> 1.3.3)
letter_opener_web (~> 1.3.0)
Expand Down
66 changes: 66 additions & 0 deletions app/models/agents/google_translation_agent.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
require 'google/apis/translate_v2'

module Agents
class GoogleTranslationAgent < Agent
cannot_be_scheduled!

description <<-MD
The Translation Agent will attempt to translate text between natural languages.

Services are provided using Google Translate. You can [sign up](https://cloud.google.com/translate/) to get `google_api_key` which is required to use this agent.

`to` must be filled with a [translator language code](https://cloud.google.com/translate/docs/languages).

`from` is the language translated from. If it's not specified, the API will attempt to detect the source language automatically and return it within the response.

Specify what you would like to translate in `content` field, you can use [Liquid](https://github.com/cantino/huginn/wiki/Formatting-Events-using-Liquid) specify which part of the payload you want to translate.

`expected_receive_period_in_days` is the maximum number of days you would allow to pass between events.
MD

event_description "User defined"

def default_options
{
'to' => "sv",
'from' => 'en',
'google_api_key' => '',
'expected_receive_period_in_days' => 1,
'content' => {
'text' => "{{message}}",
'moretext' => "{{another message}}"
}
}
end

def working?
last_receive_at && last_receive_at > interpolated['expected_receive_period_in_days'].to_i.days.ago && !recent_error_logs?
end

def validate_options
unless options['google_api_key'].present? && options['to'].present? && options['content'].present? && options['expected_receive_period_in_days'].present?
errors.add :base, "google_api_key, to, content and expected_receive_period_in_days are all required"
end
end

def translate_from
interpolated["from"].presence || 'en'
end

def receive(incoming_events)
incoming_events.each do |event|
translated_event = {}
opts = interpolated(event)
opts['content'].each_pair do |key, value|
result = translate_service.list_translations(value, opts['to'], source: translate_from)
translated_event[key] = result.translations.last.translated_text
end
create_event payload: translated_event
end
end

def translate_service
@translate_service ||= Google::Apis::TranslateV2::TranslateService.new.tap {|service| service.key = options['google_api_key']}
end
Copy link

Choose a reason for hiding this comment

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

I noticed that you memoize the method translate_service I agree it's better for performance but in our case the method has options['google_api_key'] as parameter. What do you think about using a hash with the value of options['google_api_key'] as keys in case a user changes the API key.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah it's only initialized once at startup? Then it's probably a good idea yeah.
Do you mind sharing an example of how you would do? I recently updated the code when downgrading the gem.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it should be fine, since the API key is not interpolated it can be safely memoized. The instance variable will only be set for one runtime of the Agent.

Copy link

Choose a reason for hiding this comment

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

@dsander you are right, it will be memoized for an instance scope :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍🏻

end
end
79 changes: 0 additions & 79 deletions app/models/agents/translation_agent.rb

This file was deleted.

Loading