From: "Brian C. Lane" <bcl(a)redhat.com>
The base repo needs to either be valid or None. So there is no point in
having a separate valid flag.
This renames the decorator to @refresh_base_repo, it now sets _base_repo
to None while inside the decorated methos, and refreshes it at the end.
This way any calls that check base_repo during this time will get None
and they do not depend on _yum_lock at all.
It also moves the decorator a bit. There were several places where it
was being called while inside another decorated method which doesn't
seem like a good thing to do.
This also returns to using baseRepo as a property.
---
pyanaconda/packaging/__init__.py | 5 ++-
pyanaconda/packaging/dnfpayload.py | 3 +-
pyanaconda/packaging/yumpayload.py | 80 ++++++++++++++----------------------
pyanaconda/ui/gui/spokes/network.py | 4 +-
pyanaconda/ui/gui/spokes/software.py | 2 +-
pyanaconda/ui/gui/spokes/source.py | 12 +++---
pyanaconda/ui/gui/spokes/storage.py | 2 +-
pyanaconda/ui/tui/spokes/software.py | 8 ++--
pyanaconda/ui/tui/spokes/source.py | 8 ++--
9 files changed, 53 insertions(+), 71 deletions(-)
diff --git a/pyanaconda/packaging/__init__.py b/pyanaconda/packaging/__init__.py
index a693111..b6aebb4 100644
--- a/pyanaconda/packaging/__init__.py
+++ b/pyanaconda/packaging/__init__.py
@@ -147,9 +147,10 @@ class Payload(object):
""" A list of addon repo identifiers. """
return [r.name for r in self.data.repo.dataList()]
- def getBaseRepo(self):
+ @property
+ def baseRepo(self):
"""
- Get the identifier of the current base repo.
+ Get the identifier of the current base repo. or None
"""
return None
diff --git a/pyanaconda/packaging/dnfpayload.py b/pyanaconda/packaging/dnfpayload.py
index 4db708b..0dcc41a 100644
--- a/pyanaconda/packaging/dnfpayload.py
+++ b/pyanaconda/packaging/dnfpayload.py
@@ -217,7 +217,8 @@ class DNFPayload(packaging.PackagePayload):
# addon repos via kickstart
return [r.name for r in self.data.repo.dataList()]
- def getBaseRepo(self):
+ @property
+ def baseRepo(self):
# is any locking needed here as in the yumpayload?
repo_names = [constants.BASE_REPO_NAME] + DEFAULT_REPOS
for repo in self._base.repos.iter_enabled():
diff --git a/pyanaconda/packaging/yumpayload.py b/pyanaconda/packaging/yumpayload.py
index 817c258..454a993 100644
--- a/pyanaconda/packaging/yumpayload.py
+++ b/pyanaconda/packaging/yumpayload.py
@@ -112,15 +112,16 @@ class YumLock(object):
if not isFinal:
log.debug("gave up _yum_lock for %s",
threading.currentThread().name)
-def invalidates_br_cache(cond_fn=None):
+def refresh_base_repo(cond_fn=None):
"""
- Function returning decorator for methods that invalidate base repo cache.
- After the method has been run the base repo will be re-validated
+ Function returning decorator for methods that invalidate base repo.
+ After the method has been run the base repo will be refreshed
:param cond_fn: condition function telling if base repo should be
invalidated or not (HAS TO TAKE THE SAME ARGUMENTS AS THE
DECORATED FUNCTION)
+ While the method runs the base_repo is set to None.
"""
def decorator(method):
"""
@@ -131,10 +132,13 @@ def invalidates_br_cache(cond_fn=None):
"""
@wraps(method)
def inner_method(yum_payload, *args, **kwargs):
+ if cond_fn is None or cond_fn(yum_payload, *args, **kwargs):
+ with yum_payload._base_repo_lock:
+ yum_payload._base_repo = None
+
ret = method(yum_payload, *args, **kwargs)
if cond_fn is None or cond_fn(yum_payload, *args, **kwargs):
- yum_payload.invalidate_br_cache()
yum_payload._refreshBaseRepo()
return ret
@@ -172,17 +176,11 @@ class YumPayload(PackagePayload):
self._requiredGroups = []
# base repo caching
- self._cached_base_repo = None
- self._br_cache_valid = False
- self._br_cache_valid_lock = threading.RLock()
+ self._base_repo = None
+ self._base_repo_lock = threading.RLock()
self.reset()
- def invalidate_br_cache(self):
- """ Invalidates base repo cache. """
- with self._br_cache_valid_lock:
- self._br_cache_valid = False
-
def reset(self, root=None, releasever=None):
""" Reset this instance to its initial (unconfigured) state.
"""
@@ -421,38 +419,30 @@ reposdir=%s
return _repos
- def getBaseRepo(self):
- """ Return the cached base repo id, if it is valid.
- Otherwise, re-validate it and return it.
+ @property
+ def baseRepo(self):
+ """ Return the current base repo id
+ :returns: repo id or None
- This should result in minmal access to _yum_lock, especially
- since the methods that would invalidate the cached value also
- re-validate it after they have been run. These methods are
- decorated with @invalidate_br_cache
+ Methods that change (or could change) the base_repo
+ need to be decorated with @refresh_base_repo
"""
- with self._br_cache_valid_lock:
- if self._br_cache_valid:
- return self._cached_base_repo
-
- self._refreshBaseRepo()
- return self._cached_base_repo
+ return self._base_repo
def _refreshBaseRepo(self):
+ """ Examine the enabled repos and update _base_repo
+ """
with _yum_lock:
for repo_name in BASE_REPO_NAMES:
if repo_name in self.repos and \
self._yum.repos.getRepo(repo_name).enabled:
- with self._br_cache_valid_lock:
- # cache the value for multiple use
- self._cached_base_repo = repo_name
- self._br_cache_valid = True
+ with self._base_repo_lock:
+ self._base_repo = repo_name
break
else:
# didn't find any base repo set and enabled
- with self._br_cache_valid_lock:
- # set the cache to None, but make it valid
- self._cached_base_repo = None
- self._br_cache_valid = True
+ with self._base_repo_lock:
+ self._base_repo = None
@property
def mirrorEnabled(self):
@@ -476,7 +466,7 @@ reposdir=%s
return super(YumPayload, self).isRepoEnabled(repo_id)
# pylint: disable-msg=W0221
- @invalidates_br_cache()
+ @refresh_base_repo()
def updateBaseRepo(self, fallback=True, root=None, checkmount=True):
""" Update the base repo based on self.data.method.
@@ -603,7 +593,7 @@ reposdir=%s
# if a method/repo was given, disable all default repos
self.disableRepo(repo.id)
- @invalidates_br_cache()
+ @refresh_base_repo()
def gatherRepoMetadata(self):
# now go through and get metadata for all enabled repos
log.info("gathering repo metadata")
@@ -634,6 +624,7 @@ reposdir=%s
return dev[len(DRACUT_ISODIR)+1:]
return None
+ @refresh_base_repo()
def _configureAddOnRepo(self, repo):
""" Configure a single ksdata repo. """
url = repo.baseurl
@@ -769,7 +760,6 @@ reposdir=%s
return url
- @invalidates_br_cache(lambda s, n, *args, **kwargs: n in BASE_REPO_NAMES)
def _addYumRepo(self, name, baseurl, mirrorlist=None, proxyurl=None, **kwargs):
""" Add a yum repo to the YumBase instance. """
from yum.Errors import RepoError
@@ -831,13 +821,13 @@ reposdir=%s
self._groups = None
self._packages = []
+ @refresh_base_repo(lambda s, r_id: r_id in BASE_REPO_NAMES)
def addRepo(self, newrepo):
""" Add a ksdata repo. """
log.debug("adding new repo %s", newrepo.name)
self._addYumRepo(newrepo.name, newrepo.baseurl, newrepo.mirrorlist,
newrepo.proxy) # FIXME: handle MetadataError
super(YumPayload, self).addRepo(newrepo)
- @invalidates_br_cache(lambda s, r_id: r_id in BASE_REPO_NAMES)
def _removeYumRepo(self, repo_id):
if repo_id in self.repos:
with _yum_lock:
@@ -845,13 +835,11 @@ reposdir=%s
self._groups = None
self._packages = []
+ @refresh_base_repo(lambda s, r_id: r_id in BASE_REPO_NAMES)
def removeRepo(self, repo_id):
""" Remove a repo as specified by id. """
log.debug("removing repo %s", repo_id)
- with self._br_cache_valid_lock:
- self._br_cache_valid = False
-
# if this is an NFS repo, we'll want to unmount the NFS mount after
# removing the repo
mountpoint = None
@@ -870,26 +858,18 @@ reposdir=%s
except SystemError as e:
log.error("failed to unmount nfs repo %s: %s", mountpoint, e)
- @invalidates_br_cache(lambda s, r_id: r_id in BASE_REPO_NAMES)
+ @refresh_base_repo(lambda s, r_id: r_id in BASE_REPO_NAMES)
def enableRepo(self, repo_id):
""" Enable a repo as specified by id. """
-
- with self._br_cache_valid_lock:
- self._br_cache_valid = False
-
log.debug("enabling repo %s", repo_id)
if repo_id in self.repos:
with _yum_lock:
self._yum.repos.enableRepo(repo_id)
super(YumPayload, self).enableRepo(repo_id)
- @invalidates_br_cache(lambda s, r_id: r_id in BASE_REPO_NAMES)
+ @refresh_base_repo(lambda s, r_id: r_id in BASE_REPO_NAMES)
def disableRepo(self, repo_id):
""" Disable a repo as specified by id. """
-
- with self._br_cache_valid_lock:
- self._br_cache_valid = False
-
log.debug("disabling repo %s", repo_id)
if repo_id in self.repos:
with _yum_lock:
diff --git a/pyanaconda/ui/gui/spokes/network.py b/pyanaconda/ui/gui/spokes/network.py
index 9619164..fdcaf45 100644
--- a/pyanaconda/ui/gui/spokes/network.py
+++ b/pyanaconda/ui/gui/spokes/network.py
@@ -1299,8 +1299,8 @@ class NetworkStandaloneSpoke(StandaloneSpoke):
self._now_available = self.completed
- log.debug("network standalone spoke (apply) payload: %s completed: %s",
self.payload.getBaseRepo(), self._now_available)
- if not self.payload.getBaseRepo() and not self._initially_available and
self._now_available:
+ log.debug("network standalone spoke (apply) payload: %s completed: %s",
self.payload.baseRepo, self._now_available)
+ if not self.payload.baseRepo and not self._initially_available and
self._now_available:
from pyanaconda.packaging import payloadInitialize
from pyanaconda.threads import threadMgr, AnacondaThread
diff --git a/pyanaconda/ui/gui/spokes/software.py b/pyanaconda/ui/gui/spokes/software.py
index c05fe84..b33b8c0 100644
--- a/pyanaconda/ui/gui/spokes/software.py
+++ b/pyanaconda/ui/gui/spokes/software.py
@@ -172,7 +172,7 @@ class SoftwareSelectionSpoke(NormalSpoke):
return bool(not threadMgr.get(constants.THREAD_SOFTWARE_WATCHER) and
not threadMgr.get(constants.THREAD_PAYLOAD_MD) and
not threadMgr.get(constants.THREAD_CHECK_SOFTWARE) and
- self.payload.getBaseRepo())
+ self.payload.baseRepo is not None)
@property
def showable(self):
diff --git a/pyanaconda/ui/gui/spokes/source.py b/pyanaconda/ui/gui/spokes/source.py
index d1963b6..58be9ec 100644
--- a/pyanaconda/ui/gui/spokes/source.py
+++ b/pyanaconda/ui/gui/spokes/source.py
@@ -350,7 +350,7 @@ class SourceSpoke(NormalSpoke):
# this preserves the url for later editing
self.data.method.method = None
self.data.method.proxy = self._proxyUrl
- if not old_source.method and self.payload.getBaseRepo() and \
+ if not old_source.method and self.payload.baseRepo and \
not self._proxyChange:
return False
elif self._http_active() or self._ftp_active():
@@ -457,7 +457,7 @@ class SourceSpoke(NormalSpoke):
hubQ.send_message(self.__class__.__name__, _(METADATA_DOWNLOAD_MESSAGE))
self.payload.gatherRepoMetadata()
self.payload.release()
- if not self.payload.getBaseRepo():
+ if not self.payload.baseRepo:
hubQ.send_message(self.__class__.__name__, _(METADATA_ERROR_MESSAGE))
hubQ.send_ready(self.__class__.__name__, False)
self._error = True
@@ -489,10 +489,10 @@ class SourceSpoke(NormalSpoke):
@property
def completed(self):
- if flags.automatedInstall and (not self.data.method.method or not
self.payload.getBaseRepo()):
+ if flags.automatedInstall and (not self.data.method.method or not
self.payload.baseRepo):
return False
else:
- return not self._error and self.ready and (self.data.method.method or
self.payload.getBaseRepo())
+ return not self._error and self.ready and (self.data.method.method or
self.payload.baseRepo)
@property
def mandatory(self):
@@ -511,7 +511,7 @@ class SourceSpoke(NormalSpoke):
return _("Checking software dependencies...")
elif not self.ready:
return _(BASEREPO_SETUP_MESSAGE)
- elif not self.payload.getBaseRepo():
+ elif not self.payload.baseRepo:
return _("Error setting up base repository")
elif self._error:
return _("Error setting up software source")
@@ -525,7 +525,7 @@ class SourceSpoke(NormalSpoke):
if not self._currentIsoFile:
return _("Error setting up ISO file")
return os.path.basename(self._currentIsoFile)
- elif self.payload.getBaseRepo():
+ elif self.payload.baseRepo:
return _("Closest mirror")
else:
return _("Nothing selected")
diff --git a/pyanaconda/ui/gui/spokes/storage.py b/pyanaconda/ui/gui/spokes/storage.py
index 01cf18e..d93f3a3 100644
--- a/pyanaconda/ui/gui/spokes/storage.py
+++ b/pyanaconda/ui/gui/spokes/storage.py
@@ -187,7 +187,7 @@ class InstallOptions1Dialog(GUIObject):
not threadMgr.get(constants.THREAD_PAYLOAD_MD) and
not threadMgr.get(constants.THREAD_SOFTWARE_WATCHER) and
not threadMgr.get(constants.THREAD_CHECK_SOFTWARE) and
- self.payload.getBaseRepo() is not None)
+ self.payload.baseRepo is not None)
def _check_for_storage_thread(self, button):
if self._software_is_ready():
diff --git a/pyanaconda/ui/tui/spokes/software.py b/pyanaconda/ui/tui/spokes/software.py
index 87eaefc..06534aa 100644
--- a/pyanaconda/ui/tui/spokes/software.py
+++ b/pyanaconda/ui/tui/spokes/software.py
@@ -80,7 +80,7 @@ class SoftwareSpoke(NormalTUISpoke):
return _("Error checking software selection")
if not self.ready:
return _("Processing...")
- if not self.payload.getBaseRepo():
+ if not self.payload.baseRepo:
return _("Installation source not set up")
if not self.txid_valid:
return _("Source changed - please verify")
@@ -110,15 +110,15 @@ class SoftwareSpoke(NormalTUISpoke):
not self.errors and self.txid_valid
if flags.automatedInstall:
- return processingDone and self.payload.getBaseRepo() and
self.data.packages.seen
+ return processingDone and self.payload.baseRepo and self.data.packages.seen
else:
- return self.payload.getBaseRepo() and self.environment is not None and
processingDone
+ return self.payload.baseRepo and self.environment is not None and
processingDone
def refresh(self, args=None):
""" Refresh screen. """
NormalTUISpoke.refresh(self, args)
- if not self.payload.getBaseRepo():
+ if not self.payload.baseRepo:
message = TextWidget(_("Installation source needs to be set up
first."))
self._window.append(message)
diff --git a/pyanaconda/ui/tui/spokes/source.py b/pyanaconda/ui/tui/spokes/source.py
index 565dc0f..a97176e 100644
--- a/pyanaconda/ui/tui/spokes/source.py
+++ b/pyanaconda/ui/tui/spokes/source.py
@@ -193,7 +193,7 @@ class SourceSpoke(SourceSwitchHandler, EditTUISpoke):
if not self.data.method.dir:
return _("Error setting up software source")
return os.path.basename(self.data.method.dir)
- elif self.payload.getBaseRepo():
+ elif self.payload.baseRepo:
return _("Closest mirror")
else:
return _("Nothing selected")
@@ -218,10 +218,10 @@ class SourceSpoke(SourceSwitchHandler, EditTUISpoke):
@property
def completed(self):
- if flags.automatedInstall and (not self.data.method.method or not
self.payload.getBaseRepo()):
+ if flags.automatedInstall and (not self.data.method.method or not
self.payload.baseRepo):
return False
else:
- return not self.errors and self.ready and (self.data.method.method or
self.payload.getBaseRepo())
+ return not self.errors and self.ready and (self.data.method.method or
self.payload.baseRepo)
def refresh(self, args=None):
EditTUISpoke.refresh(self, args)
@@ -316,7 +316,7 @@ class SourceSpoke(SourceSwitchHandler, EditTUISpoke):
else:
self.payload.gatherRepoMetadata()
self.payload.release()
- if not self.payload.getBaseRepo():
+ if not self.payload.baseRepo:
self.errors.append(_("Error downloading package metadata"))
else:
try:
--
1.8.3.1