From 6060da298cfb238c857f1f45707f673ca7b2a3c1 Mon Sep 17 00:00:00 2001 From: zengwei2000 <102871671+zengwei2000@users.noreply.github.com> Date: Wed, 21 Jun 2023 09:47:57 +0800 Subject: [PATCH 1/4] Combine multiple if statements to reduce Duplicate code and improve readability --- mock/py/mockbuild/backend.py | 115 +++++++++++++++++++++-------------- 1 file changed, 70 insertions(+), 45 deletions(-) diff --git a/mock/py/mockbuild/backend.py b/mock/py/mockbuild/backend.py index 6623799dd..d5c39d4f8 100644 --- a/mock/py/mockbuild/backend.py +++ b/mock/py/mockbuild/backend.py @@ -99,52 +99,77 @@ def scrub(self, scrub_opts): statestr = "scrub %s" % scrub_opts self.state.start(statestr) try: - try: - self.plugins.call_hooks('clean') + self.plugins.call_hooks('clean') + if self.bootstrap_buildroot is not None: + self.bootstrap_buildroot.plugins.call_hooks('clean') + + for scrub in scrub_opts: + self.plugins.call_hooks('scrub', scrub) if self.bootstrap_buildroot is not None: - self.bootstrap_buildroot.plugins.call_hooks('clean') - - for scrub in scrub_opts: - self.plugins.call_hooks('scrub', scrub) - if self.bootstrap_buildroot is not None: - self.bootstrap_buildroot.plugins.call_hooks('scrub', scrub) - - if scrub == 'all': - self.buildroot.root_log.info("scrubbing everything for %s", self.config_name) - self.buildroot.delete() - file_util.rmtree(self.buildroot.cachedir, selinux=self.buildroot.selinux) - if self.bootstrap_buildroot is not None: - self.bootstrap_buildroot.delete() - file_util.rmtree(self.bootstrap_buildroot.cachedir, - selinux=self.bootstrap_buildroot.selinux) - elif scrub == 'chroot': - self.buildroot.root_log.info("scrubbing chroot for %s", self.config_name) - self.buildroot.delete() - elif scrub == 'cache': - self.buildroot.root_log.info("scrubbing cache for %s", self.config_name) - file_util.rmtree(self.buildroot.cachedir, selinux=self.buildroot.selinux) - elif scrub == 'c-cache': - self.buildroot.root_log.info("scrubbing c-cache for %s", self.config_name) - file_util.rmtree(os.path.join(self.buildroot.cachedir, 'ccache'), - selinux=self.buildroot.selinux) - elif scrub == 'root-cache': - self.buildroot.root_log.info("scrubbing root-cache for %s", self.config_name) - file_util.rmtree(os.path.join(self.buildroot.cachedir, 'root_cache'), - selinux=self.buildroot.selinux) - elif scrub in ['yum-cache', 'dnf-cache']: - self.buildroot.root_log.info("scrubbing yum-cache and dnf-cache for %s", self.config_name) - file_util.rmtree(os.path.join(self.buildroot.cachedir, 'yum_cache'), - selinux=self.buildroot.selinux) - file_util.rmtree(os.path.join(self.buildroot.cachedir, 'dnf_cache'), - selinux=self.buildroot.selinux) - elif scrub == 'bootstrap' and self.bootstrap_buildroot is not None: - self.buildroot.root_log.info("scrubbing bootstrap for %s", self.config_name) - self.bootstrap_buildroot.delete() - file_util.rmtree(self.bootstrap_buildroot.cachedir, selinux=self.bootstrap_buildroot.selinux) - - except IOError as e: - getLog().warning("parts of chroot do not exist: %s", e) - raise + self.bootstrap_buildroot.plugins.call_hooks('scrub', scrub) + + scrub_actions = { + 'all': { + 'msg': "scrubbing everything for %s" % self.config_name, + 'actions': [ + lambda: self.buildroot.delete(), + lambda: file_util.rmtree(self.buildroot.cachedir, selinux=self.buildroot.selinux), + lambda: self.bootstrap_buildroot.delete(), + lambda: file_util.rmtree(self.bootstrap_buildroot.cachedir, selinux=self.bootstrap_buildroot.selinux) if self.bootstrap_buildroot is not None else None, + ] + }, + 'chroot': { + 'msg': "scrubbing chroot for %s" % self.config_name, + 'actions': [lambda: self.buildroot.delete()] + }, + 'cache': { + 'msg': "scrubbing cache for %s" % self.config_name, + 'actions': [lambda: file_util.rmtree(self.buildroot.cachedir, selinux=self.buildroot.selinux)] + }, + 'c-cache': { + 'msg': "scrubbing c-cache for %s" % self.config_name, + 'actions': [lambda: file_util.rmtree(os.path.join(self.buildroot.cachedir, 'ccache'), selinux=self.buildroot.selinux)] + }, + 'root-cache': { + 'msg': "scrubbing root-cache for %s" % self.config_name, + 'actions': [lambda: file_util.rmtree(os.path.join(self.buildroot.cachedir, 'root_cache'), selinux=self.buildroot.selinux)] + }, + 'yum-cache': { + 'msg': "scrubbing yum-cache and dnf-cache for %s" % self.config_name, + 'actions': [ + lambda: file_util.rmtree(os.path.join(self.buildroot.cachedir, 'yum_cache'), selinux=self.buildroot.selinux), + lambda: file_util.rmtree(os.path.join(self.buildroot.cachedir, 'dnf_cache'), selinux=self.buildroot.selinux) + ] + }, + 'dnf-cache': { + 'msg': "scrubbing yum-cache and dnf-cache for %s" % self.config_name, + 'actions': [ + lambda: file_util.rmtree(os.path.join(self.buildroot.cachedir, 'yum_cache'), selinux=self.buildroot.selinux), + lambda: file_util.rmtree(os.path.join(self.buildroot.cachedir, 'dnf_cache'), selinux=self.buildroot.selinux) + ] + }, + 'bootstrap': { + 'msg': "scrubbing bootstrap for %s" % self.config_name, + 'actions': [ + lambda: self.bootstrap_buildroot.delete(), + lambda: file_util.rmtree(self.bootstrap_buildroot.cachedir, selinux=self.bootstrap_buildroot.selinux) if self.bootstrap_buildroot is not None else None + ] + } + } + + try: + scrub_action = scrub_actions[scrub] + except KeyError: + raise RuntimeError("Unknown scrub option '%s'" % scrub) + + self.buildroot.root_log.info(scrub_action['msg']) + for action in scrub_action['actions']: + if action is not None: + action() + + except IOError as e: + getLog().warning("parts of chroot do not exist: %s", e) + raise finally: self.state.finish(statestr) From 3209cde8323150aaea132754481c198a92fcd5b3 Mon Sep 17 00:00:00 2001 From: zengwei2000 <102871671+zengwei2000@users.noreply.github.com> Date: Wed, 21 Jun 2023 10:05:13 +0800 Subject: [PATCH 2/4] Modify formatting issues --- mock/py/mockbuild/backend.py | 69 +++++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 28 deletions(-) diff --git a/mock/py/mockbuild/backend.py b/mock/py/mockbuild/backend.py index d5c39d4f8..842f805a6 100644 --- a/mock/py/mockbuild/backend.py +++ b/mock/py/mockbuild/backend.py @@ -95,9 +95,10 @@ def clean(self): @traceLog() def scrub(self, scrub_opts): - """clean out chroot and/or cache dirs with extreme prejudice :)""" - statestr = "scrub %s" % scrub_opts + """Clean out chroot and/or cache dirs with extreme prejudice :)""" + statestr = f"scrub {scrub_opts}" self.state.start(statestr) + try: self.plugins.call_hooks('clean') if self.bootstrap_buildroot is not None: @@ -110,65 +111,77 @@ def scrub(self, scrub_opts): scrub_actions = { 'all': { - 'msg': "scrubbing everything for %s" % self.config_name, + 'msg': f"scrubbing everything for {self.config_name}", 'actions': [ - lambda: self.buildroot.delete(), - lambda: file_util.rmtree(self.buildroot.cachedir, selinux=self.buildroot.selinux), - lambda: self.bootstrap_buildroot.delete(), - lambda: file_util.rmtree(self.bootstrap_buildroot.cachedir, selinux=self.bootstrap_buildroot.selinux) if self.bootstrap_buildroot is not None else None, + self.buildroot.delete, + lambda: file_util.rmtree(self.buildroot.cachedir, + selinux=self.buildroot.selinux), + self.bootstrap_buildroot.delete, + (lambda: file_util.rmtree(self.bootstrap_buildroot.cachedir, + selinux=self.bootstrap_buildroot.selinux) + if self.bootstrap_buildroot is not None else None), ] }, 'chroot': { - 'msg': "scrubbing chroot for %s" % self.config_name, - 'actions': [lambda: self.buildroot.delete()] + 'msg': f"scrubbing chroot for {self.config_name}", + 'actions': [self.buildroot.delete] }, 'cache': { - 'msg': "scrubbing cache for %s" % self.config_name, - 'actions': [lambda: file_util.rmtree(self.buildroot.cachedir, selinux=self.buildroot.selinux)] + 'msg': f"scrubbing cache for {self.config_name}", + 'actions': [lambda: file_util.rmtree(self.buildroot.cachedir, + selinux=self.buildroot.selinux)] }, 'c-cache': { - 'msg': "scrubbing c-cache for %s" % self.config_name, - 'actions': [lambda: file_util.rmtree(os.path.join(self.buildroot.cachedir, 'ccache'), selinux=self.buildroot.selinux)] + 'msg': f"scrubbing c-cache for {self.config_name}", + 'actions': [lambda: file_util.rmtree(os.path.join(self.buildroot.cachedir, 'ccache'), + selinux=self.buildroot.selinux)] }, 'root-cache': { - 'msg': "scrubbing root-cache for %s" % self.config_name, - 'actions': [lambda: file_util.rmtree(os.path.join(self.buildroot.cachedir, 'root_cache'), selinux=self.buildroot.selinux)] + 'msg': f"scrubbing root-cache for {self.config_name}", + 'actions': [lambda: file_util.rmtree(os.path.join(self.buildroot.cachedir, 'root_cache'), + selinux=self.buildroot.selinux)] }, 'yum-cache': { - 'msg': "scrubbing yum-cache and dnf-cache for %s" % self.config_name, + 'msg': f"scrubbing yum-cache and dnf-cache for {self.config_name}", 'actions': [ - lambda: file_util.rmtree(os.path.join(self.buildroot.cachedir, 'yum_cache'), selinux=self.buildroot.selinux), - lambda: file_util.rmtree(os.path.join(self.buildroot.cachedir, 'dnf_cache'), selinux=self.buildroot.selinux) + lambda: file_util.rmtree(os.path.join(self.buildroot.cachedir, 'yum_cache'), + selinux=self.buildroot.selinux), + lambda: file_util.rmtree(os.path.join(self.buildroot.cachedir, 'dnf_cache'), + selinux=self.buildroot.selinux) ] }, 'dnf-cache': { - 'msg': "scrubbing yum-cache and dnf-cache for %s" % self.config_name, + 'msg': f"scrubbing yum-cache and dnf-cache for {self.config_name}", 'actions': [ - lambda: file_util.rmtree(os.path.join(self.buildroot.cachedir, 'yum_cache'), selinux=self.buildroot.selinux), - lambda: file_util.rmtree(os.path.join(self.buildroot.cachedir, 'dnf_cache'), selinux=self.buildroot.selinux) + lambda: file_util.rmtree(os.path.join(self.buildroot.cachedir, 'yum_cache'), + selinux=self.buildroot.selinux), + lambda: file_util.rmtree(os.path.join(self.buildroot.cachedir, 'dnf_cache'), + selinux=self.buildroot.selinux) ] }, 'bootstrap': { - 'msg': "scrubbing bootstrap for %s" % self.config_name, + 'msg': f"scrubbing bootstrap for {self.config_name}", 'actions': [ - lambda: self.bootstrap_buildroot.delete(), - lambda: file_util.rmtree(self.bootstrap_buildroot.cachedir, selinux=self.bootstrap_buildroot.selinux) if self.bootstrap_buildroot is not None else None + self.bootstrap_buildroot.delete, + (lambda: file_util.rmtree(self.bootstrap_buildroot.cachedir, + selinux=self.bootstrap_buildroot.selinux) + if self.bootstrap_buildroot is not None else None) ] } } try: scrub_action = scrub_actions[scrub] - except KeyError: - raise RuntimeError("Unknown scrub option '%s'" % scrub) + except KeyError as exc: + raise RuntimeError(f"Unknown scrub option '{scrub}'") from exc - self.buildroot.root_log.info(scrub_action['msg']) + self.buildroot.root_log.info(f"scrubbing {scrub_action['msg']} for {self.config_name}") for action in scrub_action['actions']: if action is not None: action() except IOError as e: - getLog().warning("parts of chroot do not exist: %s", e) + self.log_manager.warning("parts of chroot do not exist: %s", e) raise finally: self.state.finish(statestr) From 32b0210fa0e350e0c74c8e72c53c11164fc71564 Mon Sep 17 00:00:00 2001 From: zengwei2000 <102871671+zengwei2000@users.noreply.github.com> Date: Wed, 21 Jun 2023 10:08:17 +0800 Subject: [PATCH 3/4] Modify formatting issues --- mock/py/mockbuild/backend.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mock/py/mockbuild/backend.py b/mock/py/mockbuild/backend.py index 842f805a6..5ebc14091 100644 --- a/mock/py/mockbuild/backend.py +++ b/mock/py/mockbuild/backend.py @@ -95,8 +95,8 @@ def clean(self): @traceLog() def scrub(self, scrub_opts): - """Clean out chroot and/or cache dirs with extreme prejudice :)""" - statestr = f"scrub {scrub_opts}" + """clean out chroot and/or cache dirs with extreme prejudice :)""" + statestr = "scrub %s" % scrub_opts self.state.start(statestr) try: @@ -181,7 +181,7 @@ def scrub(self, scrub_opts): action() except IOError as e: - self.log_manager.warning("parts of chroot do not exist: %s", e) + getLog().warning("parts of chroot do not exist: %s", e) raise finally: self.state.finish(statestr) From 07d335bc02a83f9473770f64728626d6d8f4ca99 Mon Sep 17 00:00:00 2001 From: zengwei2000 <102871671+zengwei2000@users.noreply.github.com> Date: Wed, 21 Jun 2023 10:53:26 +0800 Subject: [PATCH 4/4] Error handling when adding a network request fails --- mock/py/mockbuild/file_downloader.py | 55 ++++++++++++++++------------ 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/mock/py/mockbuild/file_downloader.py b/mock/py/mockbuild/file_downloader.py index 54f75a5b7..68755d097 100644 --- a/mock/py/mockbuild/file_downloader.py +++ b/mock/py/mockbuild/file_downloader.py @@ -23,7 +23,7 @@ def _initialize(cls): cls.tmpdir = tempfile.mkdtemp() @classmethod - def get(cls, pkg_url_or_local_file): + def get(cls, pkg_url_or_local_file, max_retry=3, timeout=10): """ If the pkg_url_or_local_file looks like a link, try to download it and store it to a temporary directory - and return path to the local @@ -32,7 +32,7 @@ def get(cls, pkg_url_or_local_file): If the pkg_url_or_local_file is not a link, do nothing - just return the pkg_url_or_local_file argument. """ - pkg = pkg_url_or_local_file + pkg = pkg_url_or_local_file # 添加这一行 log = getLog() url_prefixes = ['http://', 'https://', 'ftp://'] @@ -41,30 +41,37 @@ def get(cls, pkg_url_or_local_file): return pkg cls._initialize() - try: - url = pkg - log.info('Fetching remote file %s', url) - req = requests.get(url) - req.raise_for_status() - if req.status_code != requests.codes.ok: - log.error("Can't download {}".format(url)) - return False + try_count = 0 + while try_count < max_retry: + try: + url = pkg + log.info('Fetching remote file %s', url) + req = requests.get(url, timeout=timeout) + req.raise_for_status() - filename = urlsplit(req.url).path.rsplit('/', 1)[1] - if 'content-disposition' in req.headers: - _, params = cgi.parse_header(req.headers['content-disposition']) - if 'filename' in params and params['filename']: - filename = params['filename'] - pkg = cls.tmpdir + '/' + filename - with open(pkg, 'wb') as filed: - for chunk in req.iter_content(4096): - filed.write(chunk) - cls.backmap[pkg] = url - return pkg - except Exception as err: # pylint: disable=broad-except - log.error('Downloading error %s: %s', url, str(err)) - return None + filename = urlsplit(req.url).path.rsplit('/', 1)[1] + if 'content-disposition' in req.headers: + _, params = cgi.parse_header(req.headers['content-disposition']) + if 'filename' in params and params['filename']: + filename = params['filename'] + + pkg = cls.tmpdir + '/' + filename + with open(pkg, 'wb') as filed: + for chunk in req.iter_content(4096): + filed.write(chunk) + + cls.backmap[pkg] = url + return pkg + except requests.exceptions.RequestException as err: + try_count += 1 + log.error('Downloading error %s: %s (retrying...)', url, str(err)) + time.sleep(2 ** try_count) + except Exception as err: + log.error('Error downloading %s: %s', url, str(err)) + break + return None + @classmethod def original_name(cls, localname): """ Get the URL from the local name """