Guess what I'm getting tired of doing. --- pyanaconda/desktop.py | 2 +- pyanaconda/simpleconfig.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/pyanaconda/desktop.py b/pyanaconda/desktop.py index eaf73c5..3d73d32 100644 --- a/pyanaconda/desktop.py +++ b/pyanaconda/desktop.py @@ -49,7 +49,7 @@ class Desktop(SimpleConfigFile): super(Desktop, self).__init__() self.runlevel = 3
- def write(self, filename=None): + def write(self, filename=None, use_tmp=True): if self.getDefaultDesktop(): f = open(ROOT_PATH + "/etc/sysconfig/desktop", "w") f.write(str(self)) diff --git a/pyanaconda/simpleconfig.py b/pyanaconda/simpleconfig.py index ae2e3dc..5e474e9 100644 --- a/pyanaconda/simpleconfig.py +++ b/pyanaconda/simpleconfig.py @@ -181,7 +181,7 @@ class IfcfgFile(SimpleConfigFile): SimpleConfigFile.read(self, self.path) return len(self.info)
- def write(self, directory=None): + def write(self, directory=None, use_tmp=False): """ Writes values into ifcfg file. """
@@ -192,4 +192,4 @@ class IfcfgFile(SimpleConfigFile):
# ifcfg-rh is using inotify IN_CLOSE_WRITE event so we don't use # temporary file for new configuration - SimpleConfigFile.write(self, path, use_tmp=False) + SimpleConfigFile.write(self, path, use_tmp=use_tmp)
On 09/12/2013 10:55 AM, Chris Lumens wrote:
Guess what I'm getting tired of doing.
pyanaconda/desktop.py | 2 +- pyanaconda/simpleconfig.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/pyanaconda/desktop.py b/pyanaconda/desktop.py index eaf73c5..3d73d32 100644 --- a/pyanaconda/desktop.py +++ b/pyanaconda/desktop.py @@ -49,7 +49,7 @@ class Desktop(SimpleConfigFile): super(Desktop, self).__init__() self.runlevel = 3
- def write(self, filename=None):
- def write(self, filename=None, use_tmp=True): if self.getDefaultDesktop(): f = open(ROOT_PATH + "/etc/sysconfig/desktop", "w") f.write(str(self))
diff --git a/pyanaconda/simpleconfig.py b/pyanaconda/simpleconfig.py index ae2e3dc..5e474e9 100644 --- a/pyanaconda/simpleconfig.py +++ b/pyanaconda/simpleconfig.py @@ -181,7 +181,7 @@ class IfcfgFile(SimpleConfigFile): SimpleConfigFile.read(self, self.path) return len(self.info)
- def write(self, directory=None):
- def write(self, directory=None, use_tmp=False): """ Writes values into ifcfg file. """
@@ -192,4 +192,4 @@ class IfcfgFile(SimpleConfigFile):
# ifcfg-rh is using inotify IN_CLOSE_WRITE event so we don't use # temporary file for new configuration
SimpleConfigFile.write(self, path, use_tmp=False)
SimpleConfigFile.write(self, path, use_tmp=use_tmp)
The change in IfcfgFile is straightforward enough, but Desktop isn't using the use_tmp parameter (or filename, for that matter). Is it appropriate for it to be using write() to do what it does?
We barely use the superclass anyway. --- pyanaconda/desktop.py | 43 ++++++++++++++++++------------------------- pyanaconda/kickstart.py | 4 ++-- 2 files changed, 20 insertions(+), 27 deletions(-)
diff --git a/pyanaconda/desktop.py b/pyanaconda/desktop.py index 3d73d32..3976485 100644 --- a/pyanaconda/desktop.py +++ b/pyanaconda/desktop.py @@ -17,47 +17,40 @@ # along with this program. If not, see http://www.gnu.org/licenses/. # # Author(s): Matt Wilson msw@redhat.com +# Chris Lumens clumens@redhat.com #
import os -from simpleconfig import SimpleConfigFile from pyanaconda.constants import ROOT_PATH, RUNLEVELS
import logging log = logging.getLogger("anaconda")
-class Desktop(SimpleConfigFile): -# -# This class represents the default desktop to run and the default runlevel -# to start in -# - def setDefaultRunLevel(self, runlevel): - if int(runlevel) not in RUNLEVELS: - raise RuntimeError("Desktop::setDefaultRunLevel() - Must specify runlevel as 3 or 5!") - self.runlevel = runlevel - - def getDefaultRunLevel(self): - return self.runlevel +class Desktop(object): + def __init__(self): + self._runlevel = 3 + self.desktop = None
- def setDefaultDesktop(self, desktop): - self.info["DESKTOP"] = desktop + @property + def runlevel(self): + return self._runlevel
- def getDefaultDesktop(self): - return self.get("DESKTOP") + @runlevel.setter + def runlevel(self, runlevel): + if int(runlevel) not in RUNLEVELS: + raise RuntimeError("Desktop::setDefaultRunLevel() - Must specify runlevel as one of %s" % RUNLEVELS.keys())
- def __init__(self): - super(Desktop, self).__init__() - self.runlevel = 3 + self._runlevel = runlevel
- def write(self, filename=None, use_tmp=True): - if self.getDefaultDesktop(): - f = open(ROOT_PATH + "/etc/sysconfig/desktop", "w") - f.write(str(self)) - f.close() + def write(self): + if self.desktop: + with open(ROOT_PATH + "/etc/sysconfig/desktop", "w") as f: + f.write("DESKTOP=%s\n" % self.desktop)
if not os.path.isdir(ROOT_PATH + '/etc/systemd/system'): log.warning("there is no /etc/systemd/system directory, cannot update default.target!") return + default_target = ROOT_PATH + '/etc/systemd/system/default.target' if os.path.islink(default_target): os.unlink(default_target) diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py index 3963889..f61d2c3 100644 --- a/pyanaconda/kickstart.py +++ b/pyanaconda/kickstart.py @@ -1400,10 +1400,10 @@ class XConfig(commands.xconfig.F14_XConfig): def execute(self, *args): desktop = Desktop() if self.startX: - desktop.setDefaultRunLevel(5) + desktop.runlevel = 5
if self.defaultdesktop: - desktop.setDefaultDesktop(self.defaultdesktop) + desktop.desktop = self.defaultdesktop
# now write it out desktop.write()
On 09/12/2013 12:54 PM, Chris Lumens wrote:
We barely use the superclass anyway.
pyanaconda/desktop.py | 43 ++++++++++++++++++------------------------- pyanaconda/kickstart.py | 4 ++-- 2 files changed, 20 insertions(+), 27 deletions(-)
diff --git a/pyanaconda/desktop.py b/pyanaconda/desktop.py index 3d73d32..3976485 100644 --- a/pyanaconda/desktop.py +++ b/pyanaconda/desktop.py @@ -17,47 +17,40 @@ # along with this program. If not, see http://www.gnu.org/licenses/. # # Author(s): Matt Wilson msw@redhat.com +# Chris Lumens clumens@redhat.com #
import os -from simpleconfig import SimpleConfigFile from pyanaconda.constants import ROOT_PATH, RUNLEVELS
import logging log = logging.getLogger("anaconda")
-class Desktop(SimpleConfigFile): -# -# This class represents the default desktop to run and the default runlevel -# to start in -#
- def setDefaultRunLevel(self, runlevel):
if int(runlevel) not in RUNLEVELS:raise RuntimeError("Desktop::setDefaultRunLevel() - Must specify runlevel as 3 or 5!")self.runlevel = runlevel- def getDefaultRunLevel(self):
return self.runlevel+class Desktop(object):
- def __init__(self):
self._runlevel = 3self.desktop = None
- def setDefaultDesktop(self, desktop):
self.info["DESKTOP"] = desktop
- @property
- def runlevel(self):
return self._runlevel
- def getDefaultDesktop(self):
return self.get("DESKTOP")
- @runlevel.setter
- def runlevel(self, runlevel):
if int(runlevel) not in RUNLEVELS:raise RuntimeError("Desktop::setDefaultRunLevel() - Must specify runlevel as one of %s" % RUNLEVELS.keys())
- def __init__(self):
super(Desktop, self).__init__()self.runlevel = 3
self._runlevel = runlevel
- def write(self, filename=None, use_tmp=True):
if self.getDefaultDesktop():f = open(ROOT_PATH + "/etc/sysconfig/desktop", "w")f.write(str(self))f.close()
def write(self):
if self.desktop:with open(ROOT_PATH + "/etc/sysconfig/desktop", "w") as f:f.write("DESKTOP=%s\n" % self.desktop) if not os.path.isdir(ROOT_PATH + '/etc/systemd/system'): log.warning("there is no /etc/systemd/system directory, cannot update default.target!") returndefault_target = ROOT_PATH + '/etc/systemd/system/default.target' if os.path.islink(default_target): os.unlink(default_target)diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py index 3963889..f61d2c3 100644 --- a/pyanaconda/kickstart.py +++ b/pyanaconda/kickstart.py @@ -1400,10 +1400,10 @@ class XConfig(commands.xconfig.F14_XConfig): def execute(self, *args): desktop = Desktop() if self.startX:
desktop.setDefaultRunLevel(5)
desktop.runlevel = 5 if self.defaultdesktop:
desktop.setDefaultDesktop(self.defaultdesktop)
desktop.desktop = self.defaultdesktop # now write it out desktop.write()
Ack to both
On Thu, 2013-09-12 at 10:55 -0400, Chris Lumens wrote:
Guess what I'm getting tired of doing.
pyanaconda/desktop.py | 2 +- pyanaconda/simpleconfig.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/pyanaconda/desktop.py b/pyanaconda/desktop.py index eaf73c5..3d73d32 100644 --- a/pyanaconda/desktop.py +++ b/pyanaconda/desktop.py @@ -49,7 +49,7 @@ class Desktop(SimpleConfigFile): super(Desktop, self).__init__() self.runlevel = 3
- def write(self, filename=None):
- def write(self, filename=None, use_tmp=True): if self.getDefaultDesktop(): f = open(ROOT_PATH + "/etc/sysconfig/desktop", "w") f.write(str(self))
diff --git a/pyanaconda/simpleconfig.py b/pyanaconda/simpleconfig.py index ae2e3dc..5e474e9 100644 --- a/pyanaconda/simpleconfig.py +++ b/pyanaconda/simpleconfig.py @@ -181,7 +181,7 @@ class IfcfgFile(SimpleConfigFile): SimpleConfigFile.read(self, self.path) return len(self.info)
- def write(self, directory=None):
- def write(self, directory=None, use_tmp=False): """ Writes values into ifcfg file. """
@@ -192,4 +192,4 @@ class IfcfgFile(SimpleConfigFile):
# ifcfg-rh is using inotify IN_CLOSE_WRITE event so we don't use # temporary file for new configuration
SimpleConfigFile.write(self, path, use_tmp=False)
SimpleConfigFile.write(self, path, use_tmp=use_tmp)
I saw the pylint warning, but the problem here is a bit wider -- IfcfgFile's write method takes different parameters than SimpleConfigFile's one. The second parameter is a directory instead of a path to a file. That's why I didn't bother with adding one more never-used parameter for this "overriding" method.
We never use the directory parameter so there's no reason for IfcfgFile.write not to match SimpleConfig.write --- pyanaconda/simpleconfig.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/pyanaconda/simpleconfig.py b/pyanaconda/simpleconfig.py index 5e474e9..fe30b07 100644 --- a/pyanaconda/simpleconfig.py +++ b/pyanaconda/simpleconfig.py @@ -181,15 +181,13 @@ class IfcfgFile(SimpleConfigFile): SimpleConfigFile.read(self, self.path) return len(self.info)
- def write(self, directory=None, use_tmp=False): + def write(self, filename=None, use_tmp=False): """ Writes values into ifcfg file. """
- if not directory: - path = self.path - else: - path = os.path.join(directory, os.path.basename(self.path)) + if filename is None: + filename = self.path
# ifcfg-rh is using inotify IN_CLOSE_WRITE event so we don't use # temporary file for new configuration - SimpleConfigFile.write(self, path, use_tmp=use_tmp) + SimpleConfigFile.write(self, filename, use_tmp=use_tmp)
On 09/13/2013 09:48 AM, David Shea wrote:
We never use the directory parameter so there's no reason for IfcfgFile.write not to match SimpleConfig.write
pyanaconda/simpleconfig.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/pyanaconda/simpleconfig.py b/pyanaconda/simpleconfig.py index 5e474e9..fe30b07 100644 --- a/pyanaconda/simpleconfig.py +++ b/pyanaconda/simpleconfig.py @@ -181,15 +181,13 @@ class IfcfgFile(SimpleConfigFile): SimpleConfigFile.read(self, self.path) return len(self.info)
- def write(self, directory=None, use_tmp=False):
- def write(self, filename=None, use_tmp=False): """ Writes values into ifcfg file. """
if not directory:path = self.pathelse:path = os.path.join(directory, os.path.basename(self.path))
if filename is None:filename = self.path # ifcfg-rh is using inotify IN_CLOSE_WRITE event so we don't use # temporary file for new configuration
SimpleConfigFile.write(self, path, use_tmp=use_tmp)
SimpleConfigFile.write(self, filename, use_tmp=use_tmp)
Never mind, just saw that Radek's patch "Clean up ifcfg file handling" does the same thing
anaconda-patches@lists.fedorahosted.org