compdemocracy / polis

:milky_way: Open Source AI for large scale open ended feedback
https://pol.is
GNU Affero General Public License v3.0
781 stars 183 forks source link

migrate to npm google translate v2? #558

Open crkrenn opened 4 years ago

crkrenn commented 4 years ago

Our current authentication method for google translate is confusing, requiring 3 environment variables and a config file.

Since we are on a dev branch, are there strong objections to requiring beta users to get a new api-key?

New (https://www.npmjs.com/package/google-translate):

Require module and pass in your API Google project API key after enabling the API service. Note: This module currently doesn't support oauth authentication.

var googleTranslate = require('google-translate')(apiKey, options);

String translation:

googleTranslate.translate('My name is Brandon', 'es', function(err, translation) {
  console.log(translation.translatedText);
  // =>  Mi nombre es Brandon
});

old:

if (useTranslateApi) {
  // Tell translation library where to find credentials, and write them to disk.
  process.env.GOOGLE_APPLICATION_CREDENTIALS = '.google_creds_temp'
  // TODO: Consider deprecating GOOGLE_CREDS_STRINGIFIED in future.
  const creds_string = process.env.GOOGLE_CREDENTIALS_BASE64 ?
    new Buffer(process.env.GOOGLE_CREDENTIALS_BASE64, 'base64').toString('ascii') :
    process.env.GOOGLE_CREDS_STRINGIFIED;
  fs.writeFileSync(process.env.GOOGLE_APPLICATION_CREDENTIALS, creds_string);
  translateClient = Translate();
}
patcon commented 4 years ago

Interesting proposal!

Offering a comparison of the libraries (activity summaries, maintenance status, who maintains, etc) often helps people get behind decisions like this :)

also, you'd prob need to first submit a PR to your suggested library to get them off request, which is already deprecated (see #47). Google maintains a drop-in replacement for request that wraps node-fetch. I think it's called teensy-request

Also, the extra logic in the old version is to maintain backward compatibility on how config was stored before on pol.is, so the comparison is a bit off -- your suggested approach isn't trying to support legacy config.

I usually try to keep backward compat so it's less burden on Team Polis to consider a change. You might want to be clear about what they'd need to change on prod if they were to merge this (they use service account key files before, and this seems to be using a separate api key, so you might want to explicitly lay out how migration would happen.

Anyhow, happy to review again with more info!

patcon commented 4 years ago

Generally in favour of upgrading the translation package, even if just to the newest version of itself.

Also, seems some libraries use the Ajax api used by translate.google.com (which means no need for keys and cloud console accounts, I think). So also open to discuss supporting mulitple translation library providers, maybe like how we're doing it with email providers (also see #186). E.g., google-cloud, google-free, deepl, etc

EDIT: sorry of scope creep. I should prob create another issue, huh :)

patcon commented 4 years ago

confusing, requiring 3 environment variables and a config file.

Confusing: maybe! But did you find the docs? https://github.com/pol-is/polis/blob/dev/docs/deployment.md#enabling-comment-translation