From: Thomas Huth <thuth@redhat.com>

The fetch() function has become really huge and pylint complains about
that. Extract the internal retry-three-times-download loop into a
separate function to make it a little bit more readable and to make
pylint happy about this file again.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/functional/qemu_test/asset.py | 43 ++++++++++++++++-------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/tests/functional/qemu_test/asset.py b/tests/functional/qemu_test/asset.py
index 1cd03c2a9a9..51a434b2b70 100644
--- a/tests/functional/qemu_test/asset.py
+++ b/tests/functional/qemu_test/asset.py
@@ -133,24 +133,7 @@ def _save_time_stamp(self):
         '''
         self.cache_file.with_suffix(".stamp").write_text(f"{int(time.time())}")
 
-    def fetch(self):
-        '''Download the asset from the internet'''
-        if not self.cache_dir.exists():
-            self.cache_dir.mkdir(parents=True, exist_ok=True)
-
-        if self.valid():
-            self.log.debug("Using cached asset %s for %s",
-                           self.cache_file, self.url)
-            self._save_time_stamp()
-            return str(self.cache_file)
-
-        if not self.fetchable():
-            raise AssetError(self,
-                             "Asset cache is invalid and downloads disabled")
-
-        self.log.info("Downloading %s to %s...", self.url, self.cache_file)
-        tmp_cache_file = self.cache_file.with_suffix(".download")
-
+    def _try_to_fetch(self, tmp_cache_file):
         for _retries in range(3):
             try:
                 with tmp_cache_file.open("xb") as dst:
@@ -176,7 +159,7 @@ def fetch(self):
                                "waiting for other thread to finish...",
                                tmp_cache_file)
                 if self._wait_for_other_download(tmp_cache_file):
-                    return str(self.cache_file)
+                    return True
                 self.log.debug("%s seems to be stale, "
                                "deleting and retrying download...",
                                tmp_cache_file)
@@ -213,6 +196,28 @@ def fetch(self):
                 tmp_cache_file.unlink()
                 raise AssetError(self, f"Unable to download: {e}",
                                  transient=True) from e
+        return False
+
+    def fetch(self):
+        '''Download the asset from the internet'''
+        if not self.cache_dir.exists():
+            self.cache_dir.mkdir(parents=True, exist_ok=True)
+
+        if self.valid():
+            self.log.debug("Using cached asset %s for %s",
+                           self.cache_file, self.url)
+            self._save_time_stamp()
+            return str(self.cache_file)
+
+        if not self.fetchable():
+            raise AssetError(self,
+                             "Asset cache is invalid and downloads disabled")
+
+        self.log.info("Downloading %s to %s...", self.url, self.cache_file)
+        tmp_cache_file = self.cache_file.with_suffix(".download")
+
+        if self._try_to_fetch(tmp_cache_file):
+            return str(self.cache_file)
 
         if not os.path.exists(tmp_cache_file):
             raise AssetError(self, "Download retries exceeded", transient=True)
-- 
2.53.0

On 24/3/26 17:35, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> The fetch() function has become really huge and pylint complains about
> that. Extract the internal retry-three-times-download loop into a
> separate function to make it a little bit more readable and to make
> pylint happy about this file again.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/functional/qemu_test/asset.py | 43 ++++++++++++++++-------------
>   1 file changed, 24 insertions(+), 19 deletions(-)


> +    def _try_to_fetch(self, tmp_cache_file):
>           for _retries in range(3):

While here, pass the number as argument to the new function?

Anyhow,
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +    def fetch(self):
> +        '''Download the asset from the internet'''
> +        if not self.cache_dir.exists():
> +            self.cache_dir.mkdir(parents=True, exist_ok=True)
> +
> +        if self.valid():
> +            self.log.debug("Using cached asset %s for %s",
> +                           self.cache_file, self.url)
> +            self._save_time_stamp()
> +            return str(self.cache_file)
> +
> +        if not self.fetchable():
> +            raise AssetError(self,
> +                             "Asset cache is invalid and downloads disabled")
> +
> +        self.log.info("Downloading %s to %s...", self.url, self.cache_file)
> +        tmp_cache_file = self.cache_file.with_suffix(".download")
> +
> +        if self._try_to_fetch(tmp_cache_file):
> +            return str(self.cache_file)
>   
>           if not os.path.exists(tmp_cache_file):
>               raise AssetError(self, "Download retries exceeded", transient=True)