Issue 5777 - Update crowdin interface

localeTools.py

Index: localeTools.py
diff --git a/localeTools.py b/localeTools.py
index 140f22c1f7bb8298ce12bf7c1d4547a47de346a4..96b5fa9a6da56bf0707f1e99381a50237cafedf1 100644
--- a/localeTools.py
+++ b/localeTools.py
@@ -7,6 +7,7 @@ import os
 import sys
 import codecs
 import json
+import urlparse
 import urllib
 import urllib2
 from StringIO import StringIO
@@ -92,6 +93,47 @@ chromeLocales = [
     'zh-TW',
 ]
 
+CROWDIN_AP_URL = 'https://api.crowdin.com/api/project/{}/{}'
+
+
+def crowdin_url(project_name, action, key, get={}):

[kzar, 2017/09/28 09:48:29]

IIRC default parameter values in Python are shared, so you generally shouldn't
mutate them or use them when they might be mutated. (Same below.)

[tlucas, 2017/09/28 10:48:40]

As discussed in IRC: You are correct, but the implied problems can't occur since
'key' and 'json' get written explicitly everytime. Won't change unless someone
insists.

+    """Create a valid url for a crowdin endpoint."""
+    url = CROWDIN_AP_URL.format(project_name, action)
+    get['key'] = key
+    get['json'] = 1
+
+    scheme, netloc, path, params, query, fragment = urlparse.urlparse(url)
+
+    query = dict(urlparse.parse_qsl(query))
+    query.update(get)
+
+    return urlparse.urlunparse((
+        scheme, netloc, path, params, urllib.urlencode(query), fragment
+    ))
+
+
+def crowdin_request(project_name, action, key, get={}, post_data=None,
+                    headers={}, raw=False):
+    """Perform a call to crowdin and raise an Exception on failure."""
+    request = urllib2.Request(
+        crowdin_url(project_name, action, key, get),
+        post_data,
+        headers,
+    )
+
+    try:
+        result = urllib2.urlopen(request).read()
+    except urllib2.HTTPError as e:
+        err = e.read()
+        raise Exception(

[kzar, 2017/09/28 09:48:29]

Why catch this and raise it again, does it make the output clearer?

[tlucas, 2017/09/28 10:48:40]

It does - e.g. if urllib2 raised an HTTPError(400) this would be the only
output:

  File "/usr/lib/python2.7/urllib2.py", line 556, in http_error_default
    raise HTTPError(req.get_full_url(), code, msg, hdrs, fp)
urllib2.HTTPError: HTTP Error 400: Bad Request

compared to catching and raising again:

Exception: Server indicated that the operation was not successful
{
  "success":false,
  "error":{
    "code":13,
    "message":"Language \"foo\" is invalid or not supported"
  }
}

[kzar, 2017/09/28 11:34:44]

Cool fair enough. Mind making it display the status code too though? Knowing it
was a HTTP 400 error is actually pretty useful.

+            'Server indicated that the operation was not successful\n' + err
+        )
+
+    if not raw:
+        return json.loads(result)
+
+    return result
+
 
 class OrderedDict(dict):
     def __init__(self):
@@ -297,23 +339,22 @@ def setupTranslations(localeConfig, projectName, key):
                     locales.add(mapLocale('BCP-47', match2.group(1)))
 
     allowed = set()
-    allowedLocales = json.load(urllib2.urlopen(
-        'https://crowdin.com/languages/languages_list?callback='
-    ))
+    allowedLocales = crowdin_request(projectName, 'supported-languages', key)
+
     for locale in allowedLocales:
-        allowed.add(locale['code'])
+        allowed.add(locale['crowdin_code'])
     if not allowed.issuperset(locales):
         print "Warning, following locales aren't allowed by server: " + ', '.join(locales - allowed)
 
     locales = list(locales & allowed)
     locales.sort()
     params = urllib.urlencode([('languages[]', locale) for locale in locales])
-    result = urllib2.urlopen('http://api.crowdin.net/api/project/%s/edit-project?key=%s' % (projectName, key), params).read()
-    if result.find('<success') < 0:
-        raise Exception('Server indicated that the operation was not successful\n' + result)
 
+    crowdin_request(projectName, 'edit-project', key, post_data=params)
 
-def postFiles(files, url):
+
+def crowdin_body_headers(files):
+    """Create a post body and according headers, which Crowdin can handle."""
     boundary = '----------ThIs_Is_tHe_bouNdaRY_$'
     body = ''
     for file, data in files:
@@ -325,16 +366,17 @@ def postFiles(files, url):
     body += '--%s--\r\n' % boundary
 
     body = body.encode('utf-8')
-    request = urllib2.Request(url, StringIO(body))
-    request.add_header('Content-Type', 'multipart/form-data; boundary=%s' % boundary)
-    request.add_header('Content-Length', len(body))
-    result = urllib2.urlopen(request).read()
-    if result.find('<success') < 0:
-        raise Exception('Server indicated that the operation was not successful\n' + result)
+    return (
+        StringIO(body),
+        {
+            'Content-Type': 'multipart/form-data; ; charset=utf-8; boundary=' + boundary,

[kzar, 2017/09/28 09:48:29]

This line seems too long, does this code pass linting?

[tlucas, 2017/09/28 10:48:40]

It's about time to get rid of tox.ini 'ignore' entries... Done.

+            'Content-Length': len(body)
+        }
+    )
 
 
 def updateTranslationMaster(localeConfig, metadata, dir, projectName, key):
-    result = json.load(urllib2.urlopen('http://api.crowdin.net/api/project/%s/info?key=%s&json=1' % (projectName, key)))
+    result = crowdin_request(projectName, 'info', key)
 
     existing = set(map(lambda f: f['name'], result['files']))
     add = []
@@ -362,14 +404,18 @@ def updateTranslationMaster(localeConfig, metadata, dir, projectName, key):
                     add.append((newName, data))
 
     if len(add):
-        titles = urllib.urlencode([('titles[%s]' % name, re.sub(r'\.json', '', name)) for name, data in add])
-        postFiles(add, 'http://api.crowdin.net/api/project/%s/add-file?key=%s&type=chrome&%s' % (projectName, key, titles))
+        data = {'titles[{}]'.format(name): re.sub(r'\.json', '', name)
+                for name, data in add}
+        data['type'] = 'chrome'
+        data, headers = crowdin_body_headers(add)
+        crowdin_request(projectName, 'add-file', key, post_data=data,
+                        headers=headers)
     if len(update):
-        postFiles(update, 'http://api.crowdin.net/api/project/%s/update-file?key=%s' % (projectName, key))
+        data, headers = crowdin_body_headers(update)
+        crowdin_request(projectName, 'update-file', key, post_data=data,
+                        headers=headers)
     for file in existing:
-        result = urllib2.urlopen('http://api.crowdin.net/api/project/%s/delete-file?key=%s&file=%s' % (projectName, key, file)).read()
-        if result.find('<success') < 0:
-            raise Exception('Server indicated that the operation was not successful\n' + result)
+        crowdin_request(projectName, 'delete-file', key, {'file': file})
 
 
 def uploadTranslations(localeConfig, metadata, dir, locale, projectName, key):
@@ -392,17 +438,18 @@ def uploadTranslations(localeConfig, metadata, dir, locale, projectName, key):
             if data:
                 files.append((newName, data))
     if len(files):
-        postFiles(files, 'http://api.crowdin.net/api/project/%s/upload-translation?key=%s&language=%s' % (
-            projectName, key, mapLocale(localeConfig['name_format'], locale))
-        )
+        language = mapLocale(localeConfig['name_format'], locale)
+        data, headers = crowdin_body_headers(files)
+        crowdin_request(projectName, 'upload-translation', key,
+                        {'language': language}, post_data=data,
+                        headers=headers)
 
 
 def getTranslations(localeConfig, projectName, key):
-    result = urllib2.urlopen('http://api.crowdin.net/api/project/%s/export?key=%s' % (projectName, key)).read()
-    if result.find('<success') < 0:
-        raise Exception('Server indicated that the operation was not successful\n' + result)
+    # let crowdin build the project

[kzar, 2017/09/28 09:48:30]

Maybe this should be a docstring for the function instead?

[tlucas, 2017/09/28 10:48:40]

Done.

+    crowdin_request(projectName, 'export', key)
 
-    result = urllib2.urlopen('http://api.crowdin.net/api/project/%s/download/all.zip?key=%s' % (projectName, key)).read()
+    result = crowdin_request(projectName, 'download/all.zip', key, raw=True)
     zip = ZipFile(StringIO(result))
     dirs = {}