Closed
Bug 1091469
Opened 10 years ago
Closed 10 years ago
Use the Google Play API to update Listing Copy and Feature Descriptions
Categories
(Marketing :: Copy, task)
Marketing
Copy
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: Sylvestre, Assigned: Sylvestre)
References
Details
Attachments
(2 files, 3 obsolete files)
(deleted),
patch
|
bhearsum
:
review-
|
Details | Diff | Splinter Review |
(deleted),
patch
|
bhearsum
:
review+
|
Details | Diff | Splinter Review |
Just like we are doing in bug 1045531 (use of the Google play API to push the APK), we should use this API to update the description from the l10n platform.
Comment 1•10 years ago
|
||
(In reply to Sylvestre Ledru [:sylvestre] from comment #0)
> Just like we are doing in bug 1045531 (use of the Google play API to push
> the APK), we should use this API to update the description from the l10n
> platform.
I don't think I'm the right person to be assigned to this as I don't know what any of that means.
Assignee | ||
Comment 2•10 years ago
|
||
My bad, I did a lazy clone and forgot to update the field. I am probably going to do it.
Assignee: Mnovak → sledru
Assignee | ||
Comment 3•10 years ago
|
||
So, I have a working patch feeding from Pascal's interface:
https://l10n.mozilla-community.org/~pascalc/google_play_description/
However, we (Pascal and I) noticed that Google play:
* does not manage a bunch of locales "an", "as", "bn-IN", "cy", "es-MX", "eu", "ff", "fy-NL", "ga-IE", "gd", "gl", "gu-IN", "hy-AM", "is", "kk", "kn", "ml", "mr", "or", "pa-IN", "sq", "ta", "te"
* are not using the same language codes as us.
Here is a (bad) mapping:
if locale == "cs":
locale = "cs-CZ"
if locale == "da":
locale = "da-DK"
if locale == "de":
locale = "de-DE"
if locale == "fr":
locale = "fr-FR"
if locale == "es-AR":
locale = "es-419"
if locale == "fi":
locale = "fi-FI"
if locale == "hu":
locale = "hu-HU"
if locale == "it":
locale = "it-IT"
if locale == "ja":
locale = "ja-JP"
if locale == "ko":
locale = "ko-KR"
if locale == "nl":
locale = "nl-NL"
if locale == "pl":
locale = "pl-PL"
if locale == "ru":
locale = "ru-RU"
if locale == "tr":
locale = "tr-TR"
This is working (I have been playing with the Firefox aurora application which is disabled).
Assignee | ||
Comment 4•10 years ago
|
||
I will clean up my code and propose a patch.
Assignee | ||
Comment 5•10 years ago
|
||
Assignee | ||
Comment 6•10 years ago
|
||
Depends on bug 1045531 because it provides the authentication.
Works with:
python update_apk_description.py --package-name org.mozilla.fennec_aurora --credentials googleplay.p12 --service-account b@developer.gserviceaccount.com --update-apk-description
Comment 7•10 years ago
|
||
Comment on attachment 8516643 [details] [diff] [review]
update-description-apk.diff
Review of attachment 8516643 [details] [diff] [review]:
-----------------------------------------------------------------
::: scripts/update_apk_description.py
@@ +99,5 @@
> + response = urllib2.urlopen(ALL_LOCALES_URL)
> + return json.load(response)
> +
> + def locale_mapping(self, locale):
> + if locale == "cs":
A dictionary would make things a bit more readable
mappings = {
'cs': "cs-CZ",
"da": "da-DK",
etc.
}
if locale in mappings:
return mappings[locale]
else:
return locale
Assignee | ||
Updated•10 years ago
|
Attachment #8516653 -
Flags: review?(bhearsum)
Assignee | ||
Comment 9•10 years ago
|
||
Using the virtual env
Attachment #8516653 -
Attachment is obsolete: true
Attachment #8516653 -
Flags: review?(bhearsum)
Attachment #8517415 -
Flags: review?(bhearsum)
Assignee | ||
Comment 10•10 years ago
|
||
So, no more hardcoded locales, pascal is providing that from his web interface.
http://demos.mozfr.org/google_play_copy/web/api/
Attachment #8518187 -
Flags: review?(bhearsum)
Assignee | ||
Updated•10 years ago
|
Attachment #8517415 -
Attachment is obsolete: true
Attachment #8517415 -
Flags: review?(bhearsum)
Comment 11•10 years ago
|
||
Comment on attachment 8518187 [details] [diff] [review]
update-description-apk.diff
Review of attachment 8518187 [details] [diff] [review]:
-----------------------------------------------------------------
This looks generally okay, just a few questions below. We're probably going to need to talk about how to integrate this and bug 1045531 with the release automation at some point, too, since just landing this script won't make anything happen automatically...
::: scripts/update_apk_description.py
@@ +17,5 @@
> +from mozharness.base.script import BaseScript
> +from mozharness.mozilla.googleplay import GooglePlayMixin
> +from mozharness.base.python import VirtualenvMixin
> +
> +BASE_URL = "http://demos.mozfr.org/google_play_copy/web/"
This shouldn't be hardcoded, please make it configurable. It's fine to use this value as the default, though.
@@ +55,5 @@
> +
> + # We have 3 apps. Make sure that their names are correct
> + package_name_values = ("org.mozilla.fennec_aurora",
> + "org.mozilla.firefox_beta",
> + "org.mozilla.firefox")
What happens if we try to submit something that's not one of these three values?
@@ +158,5 @@
> + 'title': 'Firefox Aurora'}).execute()
> +
> + except client.AccessTokenRefreshError:
> + self.log('The credentials have been revoked or expired,'
> + 'please re-run the application to re-authorize')
Is it worthwhile to automatically retry this, or does it require human intervention?
Attachment #8518187 -
Flags: review?(bhearsum) → review-
Assignee | ||
Comment 12•10 years ago
|
||
Thanks for the "URL change idea", much better now!
> since just landing this script won't make anything happen automatically...
Indeed, I think we have to discuss with Pascal and Arcadio. I don't know how often we want to run that...
> What happens if we try to submit something that's not one of these three values?
fatal:
if self.config['package_name'] not in self.package_name_values:
self.fatal("Unknown package name value " +
self.config['package_name'])
> Is it worthwhile to automatically retry this, or does it require human intervention?
I see three potential failures:
* Google is down
* the arguments are incorrect (no p12 file, wrong credentials)
* we used our API quota (unlikely thing it is something like 100 000 operations per day).
So, I don't think we should do it.
Attachment #8522337 -
Flags: review?(bhearsum)
Updated•10 years ago
|
Attachment #8522337 -
Attachment is patch: true
Updated•10 years ago
|
Attachment #8522337 -
Flags: review?(bhearsum) → review+
Assignee | ||
Comment 13•10 years ago
|
||
Comment on attachment 8522337 [details] [diff] [review]
attachment.cgi?id=8518187
merged:
https://hg.mozilla.org/build/mozharness/rev/ca641817e1a4
I did a minor change (Pascal updated the json file).
Assignee | ||
Comment 14•10 years ago
|
||
This patch might have to be updated if we decide to use different strings for the different channels.
Comment 15•10 years ago
|
||
Currently, only the release channel is implemented in the l10n API. If we do localize Aurora and Beta descriptions in the future, I will implement the needed API but I don't have yet that information and I prefer not to add complexity to the API for requests not defined yet by marketing.
I documented how to use these future APIs there:
https://l10n.mozilla-community.org/~pascalc/google_play_description/api/
By default, if the channel is not specified, it is the release channel.
Comment 16•10 years ago
|
||
In production: https://hg.mozilla.org/build/mozharness/rev/ca641817e1a4
Assignee | ||
Comment 17•10 years ago
|
||
I created a new bug (bug 1110115) to implement comment #15.
Assignee | ||
Comment 18•10 years ago
|
||
So, we are running the update of the Google play description of Firefox release every day a 14:00 Paris time.
We will do the same for beta later.
You need to log in
before you can comment on or make changes to this bug.
Description
•