[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [master] dispatcher: do not show install steps in upgrade.



Looks good, I have just two small comments...

On Wed, 2011-08-10 at 10:29 +0200, Ales Kozumplik wrote:
> This works by unscheduling all the steps that haven't been done, skipped
> or requested yet. Correctly remembers the scheduling change so going back
> through the upgrade screen and selecting fresh install the next time would
> work as expected as far as Dispatcher is concerned.
> 
> Unit tests included.
> 
> Resolves: rhbz#729558
> ---
>  pyanaconda/dispatch.py                 |   16 ++++++++++++++--
>  pyanaconda/upgrade.py                  |    5 +++--
>  tests/pyanaconda_test/dispatch_test.py |   31 +++++++++++++++++++++++++++++++
>  3 files changed, 48 insertions(+), 4 deletions(-)
> 
> diff --git a/pyanaconda/dispatch.py b/pyanaconda/dispatch.py
> index 926d2fa..816666c 100644
> --- a/pyanaconda/dispatch.py
> +++ b/pyanaconda/dispatch.py
> @@ -69,7 +69,7 @@ class Step(object):
>          # allowed.
>          # unsch sched  skip   req    done
>          [True , True , True , True , True ], # unscheduled
> -        [False, True , True , True , True ], # scheduled
> +        [True,  True , True , True , True ], # scheduled
>          [False, False, True , False, False], # skipped
>          [False, False, False, True , True ], # requested
>          [False, False, False, False, True ]] # done
> @@ -97,7 +97,8 @@ class Step(object):
>                   self.namesched(self._sched),
>                   self.namesched(to_sched)))
>          self._sched = to_sched
> -        if current_step:
> +        # only track scheduling if we are in a step and if something changes:
> +        if current_step and s_from != self._sched:
>              current_step.record_history(self, s_from, self.sched)
>  
>      @property
> @@ -117,6 +118,9 @@ class Step(object):
>      def request(self, current_step):
>          return self._reschedule(self.SCHED_REQUESTED, current_step)
>  
> +    def unschedule(self, current_step):
> +        return self._reschedule(self.SCHED_UNSCHEDULED, current_step)
> +
>      def namesched(self, sched):
>          return {
>              self.SCHED_UNSCHEDULED : "unscheduled",
> @@ -302,6 +306,14 @@ class Dispatcher(object):
>              except errors.DispatchError as e:
>                  log.debug("dispatch: %s" % e)
>  
> +    def reset_scheduling(self):
> +        log.info("dispatch: resetting scheduling")
> +        for step in self.steps.keys():

You don't need to call keys() when traversing dictionary keys.

> +            try:
> +                self.steps[step].unschedule(self._current_step())

The name "_current_step" sounds more like a property,
if it is a method, wouldn't it be better to call it
"_get_current_step" ?

> +            except errors.DispatchError as e:
> +                log.debug("dispatch: %s" % e)
> +
>      def run(self):
>          self.anaconda.intf.run(self.anaconda)
>          log.info("dispatch: finished.")
> diff --git a/pyanaconda/upgrade.py b/pyanaconda/upgrade.py
> index 2e6209e..b3ce420 100644
> --- a/pyanaconda/upgrade.py
> +++ b/pyanaconda/upgrade.py
> @@ -258,8 +258,9 @@ def upgradeMountFilesystems(anaconda):
>  
>  def setSteps(anaconda):
>      dispatch = anaconda.dispatch
> -    # in case we are scheduling steps from the examine GUI, it is already too
> -    # late for some of them:
> +    dispatch.reset_scheduling() # scrap what is scheduled
> +    # in case we are scheduling steps from the examine GUI, some of them are
> +    # already done:
>      dispatch.schedule_steps_gently(
>                  "language",
>                  "keyboard",
> diff --git a/tests/pyanaconda_test/dispatch_test.py b/tests/pyanaconda_test/dispatch_test.py
> index 57de692..be10267 100644
> --- a/tests/pyanaconda_test/dispatch_test.py
> +++ b/tests/pyanaconda_test/dispatch_test.py
> @@ -70,6 +70,18 @@ class StepTest(mock.TestCase):
>          s.schedule(None)
>          self.assertEquals(s.sched, Step.SCHED_SCHEDULED)
>  
> +    def unschedule_test(self):
> +        from pyanaconda.dispatch import Step
> +        from pyanaconda.errors import DispatchError
> +        s = Step("a_step")
> +        s.schedule(None)
> +        self.assertEquals(s.sched, Step.SCHED_SCHEDULED)
> +        s.unschedule(None)
> +        self.assertEquals(s.sched, Step.SCHED_UNSCHEDULED)
> +        s.request(None)
> +        self.assertEquals(s.sched, Step.SCHED_REQUESTED)
> +        self.assertRaises(DispatchError, s.unschedule, None)
> +
>      def skip_test(self):
>          from pyanaconda.dispatch import Step
>          from pyanaconda.errors import DispatchError
> @@ -205,3 +217,22 @@ class DispatchTest(mock.TestCase):
>          self.assertEqual(d.steps["filtertype"].sched, Step.SCHED_SCHEDULED)
>          self.assertEqual(d.steps["filter"].sched, Step.SCHED_SCHEDULED)
>          self.assertDictEqual(d.steps[d.step].changes, {})
> +
> +    def reset_scheduling_test(self):
> +        from pyanaconda.dispatch import Step
> +        d = self._getDispatcher()
> +        # initial setup
> +        d.schedule_steps("betanag", "filtertype")
> +        d.request_steps("filter")
> +        # in step betanag scheduling gets reset:
> +        d.step = "betanag"
> +        d.reset_scheduling()
> +        # what is requested can not be unrequested:
> +        self.assertEqual(d.steps["betanag"].sched, Step.SCHED_UNSCHEDULED)
> +        self.assertEqual(d.steps["filtertype"].sched, Step.SCHED_UNSCHEDULED)
> +        self.assertEqual(d.steps["filter"].sched, Step.SCHED_REQUESTED)
> +        # make sure the tracking works fine
> +        self.assertEqual(
> +            d.steps["betanag"].changes,
> +            {"betanag" : (Step.SCHED_SCHEDULED, Step.SCHED_UNSCHEDULED),
> +             "filtertype" : (Step.SCHED_SCHEDULED, Step.SCHED_UNSCHEDULED)})

-- 
Martin Gracik <mgracik redhat com>


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]