From cf3e28786cf829bc042226283399699195e21d79 Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Sun, 12 Jun 2016 00:34:20 +0200 Subject: [PATCH 1/9] [mod] remove useless line --- src/yunohost/app.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index caa38e95b..0a7c92947 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -108,7 +108,6 @@ def app_fetchlist(url=None, name=None): # Rename fetched temp list os.rename('%s.tmp' % list_file, list_file) - os.system("touch /etc/cron.d/yunohost-applist-%s" % name) os.system("echo '00 00 * * * root yunohost app fetchlist -u %s -n %s > /dev/null 2>&1' >/etc/cron.d/yunohost-applist-%s" % (url, name, name)) logger.success(m18n.n('appslist_fetched')) From c4cecfcea5f51f1f9fb410358386eb5a6782cdb2 Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Sun, 12 Jun 2016 00:35:23 +0200 Subject: [PATCH 2/9] [mod] use python instead of os.system --- src/yunohost/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 0a7c92947..291d8c77a 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -108,7 +108,7 @@ def app_fetchlist(url=None, name=None): # Rename fetched temp list os.rename('%s.tmp' % list_file, list_file) - os.system("echo '00 00 * * * root yunohost app fetchlist -u %s -n %s > /dev/null 2>&1' >/etc/cron.d/yunohost-applist-%s" % (url, name, name)) + open("/etc/cron.d/yunohost-applist-%s" % name, "w").write('00 00 * * * root yunohost app fetchlist -u %s -n %s > /dev/null 2>&1\n' % (url, name)) logger.success(m18n.n('appslist_fetched')) From d9081bddef1b2129ad42b05b28a26cc7680f7d51 Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Sun, 12 Jun 2016 00:41:32 +0200 Subject: [PATCH 3/9] [mod] directly use python to retreive json list --- src/yunohost/app.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 291d8c77a..3b2a6d24f 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -36,6 +36,7 @@ import urlparse import errno import subprocess from collections import OrderedDict +from urllib import urlretrieve from moulinette.core import MoulinetteError from moulinette.utils.log import getActionLogger @@ -100,14 +101,13 @@ def app_fetchlist(url=None, name=None): raise MoulinetteError(errno.EINVAL, m18n.n('custom_appslist_name_required')) - list_file = '%s/%s.json' % (repo_path, name) - if os.system('wget "%s" -O "%s.tmp"' % (url, list_file)) != 0: - os.remove('%s.tmp' % list_file) + try: + urlretrieve(url, '%s/%s.json' % (repo_path, name)) + except Exception as e: + # I don't know how to put e into the MoulinetteError stuff + print e raise MoulinetteError(errno.EBADR, m18n.n('appslist_retrieve_error')) - # Rename fetched temp list - os.rename('%s.tmp' % list_file, list_file) - open("/etc/cron.d/yunohost-applist-%s" % name, "w").write('00 00 * * * root yunohost app fetchlist -u %s -n %s > /dev/null 2>&1\n' % (url, name)) logger.success(m18n.n('appslist_fetched')) From 97128d7d636836068ad6353f331d051121023136 Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Sun, 12 Jun 2016 00:43:17 +0200 Subject: [PATCH 4/9] [mod] exception should only be used for exceptional situations and not when buildin functions allow you to do the expected stuff --- src/yunohost/app.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 3b2a6d24f..98beca078 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -90,8 +90,8 @@ def app_fetchlist(url=None, name=None): """ # Create app path if not exists - try: os.listdir(repo_path) - except OSError: os.makedirs(repo_path) + if not os.path.exists(repo_path): + os.makedirs(repo_path) if url is None: url = 'https://app.yunohost.org/official.json' From 2aab7bdf1bcc6f025c7c5bf618d0402439abd0f4 Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Sun, 12 Jun 2016 00:44:25 +0200 Subject: [PATCH 5/9] [mod] simplify code --- src/yunohost/app.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 98beca078..f6f8699a4 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -96,10 +96,9 @@ def app_fetchlist(url=None, name=None): if url is None: url = 'https://app.yunohost.org/official.json' name = 'yunohost' - else: - if name is None: - raise MoulinetteError(errno.EINVAL, - m18n.n('custom_appslist_name_required')) + elif name is None: + raise MoulinetteError(errno.EINVAL, + m18n.n('custom_appslist_name_required')) try: urlretrieve(url, '%s/%s.json' % (repo_path, name)) From 99f0f761a5e2737b55f9f8b6ce6094b5fd7fb1ca Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Sun, 12 Jun 2016 13:21:58 +0200 Subject: [PATCH 6/9] [mod] include execption into appslist_retrieve_error message --- locales/en.json | 2 +- src/yunohost/app.py | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/locales/en.json b/locales/en.json index e939b26fa..5dd46485b 100644 --- a/locales/en.json +++ b/locales/en.json @@ -30,7 +30,7 @@ "app_upgraded": "{app:s} has been upgraded", "appslist_fetched": "The app list has been fetched", "appslist_removed": "The app list has been removed", - "appslist_retrieve_error": "Unable to retrieve the remote app list", + "appslist_retrieve_error": "Unable to retrieve the remote app list: {error}", "appslist_unknown": "Unknown app list", "ask_current_admin_password": "Current administration password", "ask_email": "Email address", diff --git a/src/yunohost/app.py b/src/yunohost/app.py index f6f8699a4..211009a04 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -103,9 +103,7 @@ def app_fetchlist(url=None, name=None): try: urlretrieve(url, '%s/%s.json' % (repo_path, name)) except Exception as e: - # I don't know how to put e into the MoulinetteError stuff - print e - raise MoulinetteError(errno.EBADR, m18n.n('appslist_retrieve_error')) + raise MoulinetteError(errno.EBADR, m18n.n('appslist_retrieve_error'), error=str(e)) open("/etc/cron.d/yunohost-applist-%s" % name, "w").write('00 00 * * * root yunohost app fetchlist -u %s -n %s > /dev/null 2>&1\n' % (url, name)) From 5b006dbf0e074f4070f6832d2c64f3b306935e3f Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Mon, 6 Feb 2017 08:37:41 -0500 Subject: [PATCH 7/9] Adding info/debug message for fetchlist --- src/yunohost/app.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 211009a04..d5874d1e4 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -101,6 +101,7 @@ def app_fetchlist(url=None, name=None): m18n.n('custom_appslist_name_required')) try: + logger.info("Fetching app list '%s' from %s ...", name, url) urlretrieve(url, '%s/%s.json' % (repo_path, name)) except Exception as e: raise MoulinetteError(errno.EBADR, m18n.n('appslist_retrieve_error'), error=str(e)) From 730156dd92bbd1b0c479821ffc829e8d4f3d2019 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Mon, 6 Feb 2017 12:54:32 -0500 Subject: [PATCH 8/9] Using request insteqd of urlretrieve, to have timeout --- src/yunohost/app.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index d5874d1e4..3f2351318 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -35,8 +35,8 @@ import socket import urlparse import errno import subprocess +import requests from collections import OrderedDict -from urllib import urlretrieve from moulinette.core import MoulinetteError from moulinette.utils.log import getActionLogger @@ -100,12 +100,18 @@ def app_fetchlist(url=None, name=None): raise MoulinetteError(errno.EINVAL, m18n.n('custom_appslist_name_required')) + # Download file try: - logger.info("Fetching app list '%s' from %s ...", name, url) - urlretrieve(url, '%s/%s.json' % (repo_path, name)) + applist = requests.get(url, timeout=30).text except Exception as e: - raise MoulinetteError(errno.EBADR, m18n.n('appslist_retrieve_error'), error=str(e)) + raise MoulinetteError(errno.EBADR, m18n.n('appslist_retrieve_error', error=str(e))) + # Write app list to file + list_file = '%s/%s.json' % (repo_path, name) + with open(list_file, "w") as f: + f.write(applist) + + # Setup a cron job to re-fetch the list at midnight open("/etc/cron.d/yunohost-applist-%s" % name, "w").write('00 00 * * * root yunohost app fetchlist -u %s -n %s > /dev/null 2>&1\n' % (url, name)) logger.success(m18n.n('appslist_fetched')) From a61445c9c3d231b9248fd247a0dd3345fc0ac6df Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 7 Feb 2017 08:18:52 -0500 Subject: [PATCH 9/9] Checking for 404 error and valid json format --- locales/en.json | 1 + src/yunohost/app.py | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/locales/en.json b/locales/en.json index 6d5dfc628..0880fdddd 100644 --- a/locales/en.json +++ b/locales/en.json @@ -31,6 +31,7 @@ "appslist_fetched": "The app list has been fetched", "appslist_removed": "The app list has been removed", "appslist_retrieve_error": "Unable to retrieve the remote app list: {error}", + "appslist_retrieve_bad_format": "Retrieved file is not a valid app list", "appslist_unknown": "Unknown app list", "ask_current_admin_password": "Current administration password", "ask_email": "Email address", diff --git a/src/yunohost/app.py b/src/yunohost/app.py index f1c0bae20..5499f4b02 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -100,10 +100,22 @@ def app_fetchlist(url=None, name=None): # Download file try: - applist = requests.get(url, timeout=30).text + applist_request = requests.get(url, timeout=30) except Exception as e: raise MoulinetteError(errno.EBADR, m18n.n('appslist_retrieve_error', error=str(e))) - + + if (applist_request.status_code != 200): + raise MoulinetteError(errno.EBADR, m18n.n('appslist_retrieve_error', error="404, not found")) + + # Validate app list format + # TODO / Possible improvement : better validation for app list (check that + # json fields actually look like an app list and not any json file) + applist = applist_request.text + try: + json.loads(applist) + except ValueError, e: + raise MoulinetteError(errno.EBADR, m18n.n('appslist_retrieve_bad_format')) + # Write app list to file list_file = '%s/%s.json' % (repo_path, name) with open(list_file, "w") as f: