Code review comment for lp://staging/~tealeg/landscape-client/ui-interface-iteration3

> Tealeg, I like this iteration of the UI configurator as I feel the layout and
> prompting makes the dialog much more clear. The ability to understand that
> you will be setting up the client to be managed by landscape or not in a
> simple drop down seems to best illustrate the consequences of the available
> config actions.
>

Thanks, it's not my idea, it was just my interpretation of what was discussed. Glad you like it though.

> [1] getting an error on ./scripts/landscape-client-settings-ui exit when I
> choose any of the options:
> "Do not manage with this computer landscape"
> "Use Canonical's hosted landscape service"
> "Use dedicated landscape system"
>
>
> ./scripts/landscape-client-settings-ui
> Traceback (most recent call last):
> File "./landscape/ui/controller/app.py", line 60, in setup_ui
> self.settings_dialog.persist()
> File "./landscape/ui/view/configuration.py", line 158, in persist
> self.controller.persist()
> File "./landscape/ui/model/configuration/state.py", line 437, in persist
> self._current_state = self._current_state.persist()
> File "./landscape/ui/model/configuration/state.py", line 367, in persist
> return self._unpersistable_helper.persist()
> File "./landscape/ui/model/configuration/state.py", line 285, in persist
> " cannot be transitioned via persist().")
> landscape.ui.model.configuration.state.StateError: A ConfiguratonModel in
> UnpersistableHelper cannot be transitioned via persist().
>

Ah, yes - the call is transparently passing through the controller to the model, which is wrong. I have addressed this, although it actually goes away in a later branch anyhow. Easier to think about the branches in isolation I think.

> [2] a number of lint failures are surfaced via "make lint"
> bulk of the lint failures seem to in tests that are missing imports for
> constants from landscape/ui/model/configuration/state.py or
> landscape.ui.constants
>

Hmm, any chance you can provide output? My lint shows no errors.

>
> [3] +ping_url = http://localhost:8080/ping
> ./dev/racercar by default throws the ping server at port 8081. Any reason for
> the shift to 8080 in landscape-client.conf?

No.. it just got checked in accidentally! 2nd time that has happened, good catch!

>
>
> still have a bit more to review on this. abstaining until I apply the related
> MP that might handle some of the persist errors.
>

OK - I'll check in with you later today.

>
> If [1] is an easy fix, the settings UI appears to write the appropriate values
> to the /etc/landscape/client.conf appropriately. Just need to handle the
> error.

See above, we should be good.