Split the apply method into apply and save methods.
This came up in the meeting today, so here's a patch to start splitting
things apart so testing the UI doesn't result in your language changing.
Note that datetime and network spokes are not substantively included here.
The former looks difficult but doable, the latter looks just about impossible
given NM.
Thoughts?
- Chris
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
06-20-2012, 07:47 PM
Chris Lumens
Split the apply method into apply and save methods.
The apply methods have ended up trying to do two separate tasks at once:
setting values into ksdata, and applying changes (like keyboard or language)
to the running system. This works okay for system installation, but is not
good for testing or for future use of the UI as a kickstart generator.
Thus, this patch starts splitting the current apply methods out into two
task-specific methods. save methods will write values into the ksdata and
can optionally do stuff like download temporary files. These methods are
mandatory. apply methods will make permanent changes. These methods are
optional, as not every spoke will need to do this.
---
pyanaconda/ui/gui/hubs/__init__.py | 5 ++++-
pyanaconda/ui/gui/spokes/__init__.py | 20 ++++++++++++++++++--
pyanaconda/ui/gui/spokes/custom.py | 2 +-
pyanaconda/ui/gui/spokes/datetime_spoke.py | 2 +-
pyanaconda/ui/gui/spokes/keyboard.py | 5 ++++-
pyanaconda/ui/gui/spokes/network.py | 4 ++--
pyanaconda/ui/gui/spokes/software.py | 4 ++--
pyanaconda/ui/gui/spokes/source.py | 2 +-
pyanaconda/ui/gui/spokes/storage.py | 2 +-
pyanaconda/ui/gui/spokes/welcome.py | 10 ++++++----
10 files changed, 40 insertions(+), 16 deletions(-)
diff --git a/pyanaconda/ui/gui/hubs/__init__.py b/pyanaconda/ui/gui/hubs/__init__.py
index 40cc6ef..c996aa5 100644
--- a/pyanaconda/ui/gui/hubs/__init__.py
+++ b/pyanaconda/ui/gui/hubs/__init__.py
@@ -101,7 +101,10 @@ class Hub(UIObject):
# prevent the user from switching away. It's up to the spoke's back
# button handler to kill its own layer of main loop.
Gtk.main()
- action.apply()
+ action.save()
+
+ if not flags.testing and not flags.imageInstall:
+ action.apply()
def _createBox(self):
from gi.repository import Gtk, AnacondaWidgets
diff --git a/pyanaconda/ui/gui/spokes/__init__.py b/pyanaconda/ui/gui/spokes/__init__.py
index 3f333ba..4531775 100644
--- a/pyanaconda/ui/gui/spokes/__init__.py
+++ b/pyanaconda/ui/gui/spokes/__init__.py
@@ -86,8 +86,24 @@ class Spoke(UIObject):
self.instclass = instclass
def apply(self):
- """Apply the selections made on this Spoke to the object's preset
- data object. This method must be provided by every subclass.
+ """Immediately apply the selections made on this Spoke to the runtime
+ system. This does not do anything to the object's preset data
+ object. That is handled by the save method. This method need only
+ be provided if this object needs to set up something for the
+ installation environment (examples are things like language and
+ keyboard).
+
+ This method can assume that save has been called first, thus the data
+ object is up-to-date.
+ """
+ pass
+
+ def save(self):
+ """Take the selections made on this Spoke and save them to the object's
+ preset data object. This method does not cause any permanent changes
+ to be made to the runtime system, though it may do things like download
+ temporary files. No changes will be made to the target system. This
+ method must be provided by every subclass.
"""
raise NotImplementedError
diff --git a/pyanaconda/ui/gui/spokes/keyboard.py b/pyanaconda/ui/gui/spokes/keyboard.py
index 154a88c..951dbd4 100644
--- a/pyanaconda/ui/gui/spokes/keyboard.py
+++ b/pyanaconda/ui/gui/spokes/keyboard.py
@@ -131,11 +131,14 @@ class KeyboardSpoke(NormalSpoke):
self._xkl_wrapper = keyboard.XklWrapper.get_instance()
def apply(self):
+ # FIXME: Set the keyboard layout here
+ pass
+
+ def save(self):
# Clear and repopulate self.data with actual values
self.data.keyboard.layouts_list = list()
for row in self._store:
self.data.keyboard.layouts_list.append(row[0])
- # FIXME: Set the keyboard layout here, too.
- def apply(self):
- # NOTE: Other apply methods work directly with the ksdata, but this
+ def save(self):
+ # NOTE: Other save methods work directly with the ksdata, but this
# one does not. However, selectGroup/deselectGroup modifies ksdata as
# part of its operation. So this is fine.
from pyanaconda.threads import threadMgr, AnacondaThread
diff --git a/pyanaconda/ui/gui/spokes/source.py b/pyanaconda/ui/gui/spokes/source.py
index 9a87c03..0774c4d 100644
--- a/pyanaconda/ui/gui/spokes/source.py
+++ b/pyanaconda/ui/gui/spokes/source.py
@@ -419,7 +419,7 @@ class SourceSpoke(NormalSpoke):
self._ready = False
self._error = False
- def apply(self):
+ def save(self):
from pyanaconda.threads import threadMgr, AnacondaThread
from pyanaconda.packaging import PayloadError
import copy
diff --git a/pyanaconda/ui/gui/spokes/storage.py b/pyanaconda/ui/gui/spokes/storage.py
index 26095f5..57fb1b0 100644
--- a/pyanaconda/ui/gui/spokes/storage.py
+++ b/pyanaconda/ui/gui/spokes/storage.py
@@ -245,7 +245,7 @@ class StorageSpoke(NormalSpoke):
# FIXME: This needs to be set to a real value via some TBD UI.
self.clearPartType = CLEARPART_TYPE_LINUX
- lang = store[itr][2]
+ self.data.lang.lang = store[itr][2]
self.language.select_translation(lang)
- self.data.lang.lang = lang
#TODO: better use GeoIP data once it is available
if self.language.territory and not self.data.timezone.timezone:
@@ -81,7 +80,10 @@ class LanguageMixIn(object):
for layout in new_layouts:
if layout not in self.data.keyboard.layouts_list:
self.data.keyboard.layouts_list.append(layout)
- self._xklwrapper.add_layout(layout)
+
+ def apply(self):
+ for layout in self.data.keyboard.layouts_list:
+ self._xklwrapper.add_layout(layout)
@property
def completed(self):
--
1.7.8.4
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
06-20-2012, 08:00 PM
Will Woods
Split the apply method into apply and save methods.
On Wed, 2012-06-20 at 15:47 -0400, Chris Lumens wrote:
> The apply methods have ended up trying to do two separate tasks at once:
> setting values into ksdata, and applying changes (like keyboard or language)
> to the running system. This works okay for system installation, but is not
> good for testing or for future use of the UI as a kickstart generator.
>
> Thus, this patch starts splitting the current apply methods out into two
> task-specific methods. save methods will write values into the ksdata and
> can optionally do stuff like download temporary files. These methods are
> mandatory. apply methods will make permanent changes. These methods are
> optional, as not every spoke will need to do this.
I like the idea of each spoke having a save() method that writes changes
to the ksdata object.
But I feel like the apply() methods should be independent of the UI,
shouldn't they? In theory, applying the changes requested by ksdata (at
least for simple things like time zone) would be the same procedure
regardless of which UI requests those changes.
Maybe apply() should be a method on the Handler object(s) instead?
-w
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
06-20-2012, 08:08 PM
Chris Lumens
Split the apply method into apply and save methods.
Incidentally, my original patch was incomplete. I completely missed the
part where welcome.py will still change your language via
locale.setlocale. Clearly, more needs to be done.
> But I feel like the apply() methods should be independent of the UI,
> shouldn't they? In theory, applying the changes requested by ksdata (at
> least for simple things like time zone) would be the same procedure
> regardless of which UI requests those changes.
They are and aren't independent of the UI. It's likely the code could
be shared at least to some degree across interfaces, yes. It also needs
to be written in such a way that it can happen immediately. That's what
this is all about - some spokes have to change things in the running
environment right away.
> Maybe apply() should be a method on the Handler object(s) instead?
Hmm, that's interesting. We've already got execute methods in
kickstart.py. Those could just be beefed up, and then the apply method
on a spoke could just call the correct execute method. I still like
having apply on a spoke so it's easy to centrally set things when the
spoke is done running.
It occurs to me as I type all of this that the current behavior of the
welcome spoke is to change the language as you flip through the list,
which is completely contrary to everything else we're talking about
here. Joy.
- Chris
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
06-21-2012, 08:04 AM
Vratislav Podzimek
Split the apply method into apply and save methods.
On Wed, 2012-06-20 at 15:47 -0400, Chris Lumens wrote:
> This came up in the meeting today, so here's a patch to start splitting
> things apart so testing the UI doesn't result in your language changing.
> Note that datetime and network spokes are not substantively included here.
I think that the keyboard spoke is problematic as well. Having a text
entry to test layouts, we need to change the X runtime configuration
instantly after adding/removing/reordering layouts. On the other hand in
cases of an image or live installations, we want to preserve the user
configuration. So how to deal with this? Save the configuration in the
begining of the installation and then restore it every time our window
loses focus? This could be quite complicated per se and it would bring
another problem -- desktop environments typically have their own layer
on top of the X layouts configuration. Should we write a bunch of code
dealing with gnome-session's layouts configuration, KDE's layouts
configuration, etc.?
I believe it could be better to just make the entry for testing layouts
insensitive and do not change X runtime configuration when
adding/removing/reordering layouts in the
system-config-kickstart/live/image-install mode. But for this, the spoke
needs to know in what mode it is actually running.
The same goes for the datetime spoke. I believe everybody agrees, that
setting time and date in the system-config-kickstart/image-install (and
maybe live install as well) mode is a nonsense, so the part of this
spoke should also be (in)sensitive depending on the mode it is running
in. Starting/stopping chronyd.service when turning NTP on/off is the
same case. (btw, the timezone switching is ok as it just sets the $TZ
environment variable for the Anaconda process)
So summed up, we need to get the information about the mode to the
spokes (in a better way than testing flags.imageInstall and
flags.liveInstall). That way, I can easily patch these two spokes to not
ruin the environment when running on the live system.
I'm also thinking about some "change_runtime_exec" function in iutil,
that would actually not do the exec if running in the live/image-install
mode. Having something like that could simplify the spokes' code.
Comments are, of course, more than welcome. And once we agree on a way
how to deal with these issues, I'll write patches.
--
Vratislav Podzimek
Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
Split the apply method into apply and save methods.
