[buildstream] 01/01: Stop passing URL opener across process boundaries

[tv...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

tvb pushed a commit to branch tristan/downloadable-source
in repository https://gitbox.apache.org/repos/asf/buildstream.git

commit 63e16b7fa9ac5410cbab078387b0fd2136b87564
Author: Seppo Yli-Olli <se...@gmail.com>
AuthorDate: Sun Jan 29 17:48:00 2023 +0200

    Stop passing URL opener across process boundaries
    
    The object is not picklable. This cannot be supported. Instead create
    URL opener when needed in child process. Drops URL opener caching
    which may result in minor performance impact.
---
 src/buildstream/_stream.py                |  7 ----
 src/buildstream/downloadablefilesource.py | 61 ++++++++++++++++---------------
 2 files changed, 32 insertions(+), 36 deletions(-)

diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py
index 57c19ee1a..9372347e3 100644
--- a/src/buildstream/_stream.py
+++ b/src/buildstream/_stream.py
@@ -50,7 +50,6 @@ from ._state import State
 from .types import _KeyStrength, _PipelineSelection, _Scope, _HostMount
 from .plugin import Plugin
 from . import utils, node, _yaml, _site, _pipeline
-from .downloadablefilesource import DownloadableFileSource
 
 
 # Stream()
@@ -118,12 +117,6 @@ class Stream:
         # test isolation.
         node._reset_global_state()
 
-        # Ensure that any global state loaded by the downloadablefilesource
-        # is discarded in between sessions (different invocations of the CLI
-        # may come with different local state such as .netrc files, so we need
-        # a reset here).
-        DownloadableFileSource._reset_url_opener()
-
     # set_project()
     #
     # Set the top-level project.
diff --git a/src/buildstream/downloadablefilesource.py b/src/buildstream/downloadablefilesource.py
index 2e261cb6e..16f6604cd 100644
--- a/src/buildstream/downloadablefilesource.py
+++ b/src/buildstream/downloadablefilesource.py
@@ -88,7 +88,8 @@ class _NetrcPasswordManager:
             return login, password
 
 
-def _download_file(opener, url, etag, directory):
+def _download_file(opener_creator, url, etag, directory):
+    opener = opener_creator.get_url_opener()
     default_name = os.path.basename(url)
     request = urllib.request.Request(url)
     request.add_header("Accept", "*/*")
@@ -134,7 +135,6 @@ class DownloadableFileSource(Source):
 
     COMMON_CONFIG_KEYS = Source.COMMON_CONFIG_KEYS + ["url", "ref", "etag"]
 
-    __urlopener = None
     __default_mirror_file = None
 
     def configure(self, node):
@@ -230,8 +230,10 @@ class DownloadableFileSource(Source):
             else:
                 etag = None
 
+            url_opener_creator = _UrlOpenerCreator(self._parse_netrc())
+
             local_file, new_etag, error = self.blocking_activity(
-                _download_file, (self.__get_urlopener(), self.url, etag, td), activity_name
+                _download_file, (url_opener_creator, self.url, etag, td), activity_name
             )
 
             if error:
@@ -254,6 +256,21 @@ class DownloadableFileSource(Source):
                 self._store_etag(sha256, new_etag)
             return sha256
 
+    def _parse_netrc(self):
+        netrc_config = None
+        try:
+            netrc_config = netrc.netrc()
+        except OSError:
+            # If the .netrc file was not found, FileNotFoundError will be
+            # raised, but OSError will be raised directly by the netrc package
+            # in the case that $HOME is not set.
+            #
+            # This will catch both cases.
+            pass
+        except netrc.NetrcParseError as e:
+            self.warn("{}: While reading .netrc: {}".format(self, e))
+        return netrc_config
+
     def _get_mirror_file(self, sha=None):
         if sha is not None:
             return os.path.join(self._mirror_dir, sha)
@@ -263,29 +280,15 @@ class DownloadableFileSource(Source):
 
         return self.__default_mirror_file
 
-    @classmethod
-    def _reset_url_opener(cls):
-        # Needed for tests, in order to cleanup the `netrc` configuration.
-        cls.__urlopener = None  # pylint: disable=unused-private-member
-
-    def __get_urlopener(self):
-        if not DownloadableFileSource.__urlopener:
-            try:
-                netrc_config = netrc.netrc()
-            except OSError:
-                # If the .netrc file was not found, FileNotFoundError will be
-                # raised, but OSError will be raised directly by the netrc package
-                # in the case that $HOME is not set.
-                #
-                # This will catch both cases.
-                #
-                DownloadableFileSource.__urlopener = urllib.request.build_opener()
-            except netrc.NetrcParseError as e:
-                self.warn("{}: While reading .netrc: {}".format(self, e))
-                return urllib.request.build_opener()
-            else:
-                netrc_pw_mgr = _NetrcPasswordManager(netrc_config)
-                http_auth = urllib.request.HTTPBasicAuthHandler(netrc_pw_mgr)
-                ftp_handler = _NetrcFTPOpener(netrc_config)
-                DownloadableFileSource.__urlopener = urllib.request.build_opener(http_auth, ftp_handler)
-        return DownloadableFileSource.__urlopener
+
+class _UrlOpenerCreator:
+    def __init__(self, netrc_config):
+        self.netrc_config = netrc_config
+
+    def get_url_opener(self):
+        if self.netrc_config:
+            netrc_pw_mgr = _NetrcPasswordManager(self.netrc_config)
+            http_auth = urllib.request.HTTPBasicAuthHandler(netrc_pw_mgr)
+            ftp_handler = _NetrcFTPOpener(self.netrc_config)
+            return urllib.request.build_opener(http_auth, ftp_handler)
+        return urllib.request.build_opener()