From 0e1807091087018338134b4aa1f10a3fd58e0589 Mon Sep 17 00:00:00 2001 From: Rohan Bansal Date: Wed, 11 Nov 2020 14:56:55 +0530 Subject: [PATCH 1/6] fix: error on trying to check semantic version --- frappe/utils/change_log.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/frappe/utils/change_log.py b/frappe/utils/change_log.py index 29fee2bac0..f7fac4cdf4 100644 --- a/frappe/utils/change_log.py +++ b/frappe/utils/change_log.py @@ -165,9 +165,10 @@ def check_for_update(): add_message_to_redis(updates) + def parse_latest_non_beta_release(response): """ - Pasrses the response JSON for all the releases and returns the latest non prerelease + Parses the response JSON for all the releases and returns the latest non prerelease Parameters response (list): response object returned by github @@ -182,32 +183,34 @@ def parse_latest_non_beta_release(response): return None + def check_release_on_github(app): - # Check if repo remote is on github from subprocess import CalledProcessError + try: + # Check if repo remote is on github remote_url = subprocess.check_output("cd ../apps/{} && git ls-remote --get-url".format(app), shell=True).decode() except CalledProcessError: # Passing this since some apps may not have git initializaed in them - return None + return if isinstance(remote_url, bytes): remote_url = remote_url.decode() if "github.com" not in remote_url: - return None + return # Get latest version from github if 'https' not in remote_url: - return None + return org_name = remote_url.split('/')[3] r = requests.get('https://api.github.com/repos/{}/{}/releases'.format(org_name, app)) if r.ok: - lastest_non_beta_release = parse_latest_non_beta_release(r.json()) - return Version(lastest_non_beta_release), org_name - # In case of an improper response or if there are no releases - return None + latest_non_beta_release = parse_latest_non_beta_release(r.json()) + if latest_non_beta_release: + return Version(latest_non_beta_release), org_name + def add_message_to_redis(update_json): # "update-message" will store the update message string From b52cfd1903a050f0d3493f120d75f00c2d715168 Mon Sep 17 00:00:00 2001 From: Rohan Bansal Date: Thu, 12 Nov 2020 15:24:12 +0530 Subject: [PATCH 2/6] fix: add git URL check --- frappe/utils/change_log.py | 16 ++++++++++------ frappe/utils/data.py | 6 ++++++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/frappe/utils/change_log.py b/frappe/utils/change_log.py index f7fac4cdf4..75421c43ea 100644 --- a/frappe/utils/change_log.py +++ b/frappe/utils/change_log.py @@ -1,15 +1,16 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # MIT License. See license.txt -from __future__ import unicode_literals -from six.moves import range -import json, os -from semantic_version import Version +import json +import os +import subprocess # nosec + import frappe import requests -import subprocess # nosec -from frappe.utils import cstr from frappe import _, safe_decode +from frappe.utils import cstr, is_git_url +from semantic_version import Version +from six.moves import range def get_change_log(user=None): @@ -204,6 +205,9 @@ def check_release_on_github(app): if 'https' not in remote_url: return + if is_git_url(remote_url): + return + org_name = remote_url.split('/')[3] r = requests.get('https://api.github.com/repos/{}/{}/releases'.format(org_name, app)) if r.ok: diff --git a/frappe/utils/data.py b/frappe/utils/data.py index 41f247da45..1ddf59a82c 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -1376,3 +1376,9 @@ def validate_json_string(string): json.loads(string) except (TypeError, ValueError): raise frappe.ValidationError + + +def is_git_url(url): + # modified to allow without the tailing .git from https://github.com/jonschlinkert/is-git-url.git + pattern = r"(?:git|ssh|https?|git@[-\w.]+):(\/\/)?(.*?)(\.git)?(\/?|\#[-\d\w._]+?)$" + return bool(re.match(pattern, url)) From 08c8e517b6d484c5ad9aa1848ace855fec4c4c15 Mon Sep 17 00:00:00 2001 From: Rohan Bansal Date: Wed, 25 Nov 2020 18:08:18 +0530 Subject: [PATCH 3/6] fix: git check flow --- frappe/utils/change_log.py | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/frappe/utils/change_log.py b/frappe/utils/change_log.py index 75421c43ea..148d03ae39 100644 --- a/frappe/utils/change_log.py +++ b/frappe/utils/change_log.py @@ -185,29 +185,38 @@ def parse_latest_non_beta_release(response): return None -def check_release_on_github(app): - from subprocess import CalledProcessError +def check_release_on_github(app: str): + """ + Check the latest release for a given Frappe application hosted on Github. + + Args: + app (str): The name of the Frappe application. + + Returns: + tuple(Version, str): The Version object of the latest release and the + organization name, if the application exists, otherwise None. + """ try: # Check if repo remote is on github - remote_url = subprocess.check_output("cd ../apps/{} && git ls-remote --get-url".format(app), shell=True).decode() - except CalledProcessError: - # Passing this since some apps may not have git initializaed in them + remote_url = subprocess.check_output("cd ../apps/{} && git ls-remote --get-url".format(app), shell=True) + except subprocess.CalledProcessError: + # Passing this since some apps may not have git initialized in them return if isinstance(remote_url, bytes): remote_url = remote_url.decode() - if "github.com" not in remote_url: + if "github" not in remote_url: + return + + if is_git_url(remote_url): return # Get latest version from github if 'https' not in remote_url: return - if is_git_url(remote_url): - return - org_name = remote_url.split('/')[3] r = requests.get('https://api.github.com/repos/{}/{}/releases'.format(org_name, app)) if r.ok: From 9a84a7eb45c529d882f6b541db0916232468d783 Mon Sep 17 00:00:00 2001 From: Rohan Bansal Date: Fri, 27 Nov 2020 17:27:44 +0530 Subject: [PATCH 4/6] feat: use giturlparse to parse Git URLs --- frappe/utils/change_log.py | 32 ++++++++++++++++++++------------ frappe/utils/data.py | 6 ------ requirements.txt | 1 + 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/frappe/utils/change_log.py b/frappe/utils/change_log.py index 148d03ae39..9607c89784 100644 --- a/frappe/utils/change_log.py +++ b/frappe/utils/change_log.py @@ -5,13 +5,14 @@ import json import os import subprocess # nosec -import frappe import requests -from frappe import _, safe_decode -from frappe.utils import cstr, is_git_url from semantic_version import Version from six.moves import range +import frappe +from frappe import _, safe_decode +from frappe.utils import cstr + def get_change_log(user=None): if not user: user = frappe.session.user @@ -193,10 +194,13 @@ def check_release_on_github(app: str): app (str): The name of the Frappe application. Returns: - tuple(Version, str): The Version object of the latest release and the + tuple(Version, str): The semantic version object of the latest release and the organization name, if the application exists, otherwise None. """ + from giturlparse import parse + from giturlparse.parser import ParserError + try: # Check if repo remote is on github remote_url = subprocess.check_output("cd ../apps/{} && git ls-remote --get-url".format(app), shell=True) @@ -207,22 +211,26 @@ def check_release_on_github(app: str): if isinstance(remote_url, bytes): remote_url = remote_url.decode() - if "github" not in remote_url: + try: + parsed_url = parse(remote_url) + except ParserError: + # Invalid URL return - if is_git_url(remote_url): + # Get latest version from Github + if parsed_url.protocol == "http": + return + if parsed_url.resource != "github.com": return - # Get latest version from github - if 'https' not in remote_url: - return + owner = parsed_url.owner + repo = parsed_url.name - org_name = remote_url.split('/')[3] - r = requests.get('https://api.github.com/repos/{}/{}/releases'.format(org_name, app)) + r = requests.get('https://api.github.com/repos/{}/{}/releases'.format(owner, repo)) if r.ok: latest_non_beta_release = parse_latest_non_beta_release(r.json()) if latest_non_beta_release: - return Version(latest_non_beta_release), org_name + return Version(latest_non_beta_release), owner def add_message_to_redis(update_json): diff --git a/frappe/utils/data.py b/frappe/utils/data.py index cb61355a29..34659e1cac 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -1397,9 +1397,3 @@ def validate_json_string(string): json.loads(string) except (TypeError, ValueError): raise frappe.ValidationError - - -def is_git_url(url): - # modified to allow without the tailing .git from https://github.com/jonschlinkert/is-git-url.git - pattern = r"(?:git|ssh|https?|git@[-\w.]+):(\/\/)?(.*?)(\.git)?(\/?|\#[-\d\w._]+?)$" - return bool(re.match(pattern, url)) diff --git a/requirements.txt b/requirements.txt index de9e675a67..59c4a9dbf8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -15,6 +15,7 @@ Faker==2.0.4 future==0.18.2 gitdb2==2.0.6;python_version<'3.4' GitPython==2.1.15 +git-url-parse==1.2.2 google-api-python-client==1.9.3 google-auth-httplib2==0.0.3 google-auth-oauthlib==0.4.1 From f8fd59b6eb26b03c324fe06f0ed10409ae5ac0a4 Mon Sep 17 00:00:00 2001 From: Rohan Bansal Date: Mon, 30 Nov 2020 15:15:19 +0530 Subject: [PATCH 5/6] fix: remove HTTP filter --- frappe/utils/change_log.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/frappe/utils/change_log.py b/frappe/utils/change_log.py index 9607c89784..7cea1554b4 100644 --- a/frappe/utils/change_log.py +++ b/frappe/utils/change_log.py @@ -217,15 +217,13 @@ def check_release_on_github(app: str): # Invalid URL return - # Get latest version from Github - if parsed_url.protocol == "http": - return if parsed_url.resource != "github.com": return owner = parsed_url.owner repo = parsed_url.name + # Get latest version from Github r = requests.get('https://api.github.com/repos/{}/{}/releases'.format(owner, repo)) if r.ok: latest_non_beta_release = parse_latest_non_beta_release(r.json()) From 3e1f6c8103fd705c67968d5b1c9f46867af0ecd5 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 30 Nov 2020 15:37:29 +0530 Subject: [PATCH 6/6] style: Use f-string instead of format --- frappe/utils/change_log.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/utils/change_log.py b/frappe/utils/change_log.py index 7cea1554b4..33801af722 100644 --- a/frappe/utils/change_log.py +++ b/frappe/utils/change_log.py @@ -223,8 +223,8 @@ def check_release_on_github(app: str): owner = parsed_url.owner repo = parsed_url.name - # Get latest version from Github - r = requests.get('https://api.github.com/repos/{}/{}/releases'.format(owner, repo)) + # Get latest version from GitHub + r = requests.get(f"https://api.github.com/repos/{owner}/{repo}/releases") if r.ok: latest_non_beta_release = parse_latest_non_beta_release(r.json()) if latest_non_beta_release: