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

Re: [virt-tools-list] [PATCH] Drop user back to 'open conn' dialog if connecting fails



Cole Robinson <crobinso redhat com> writes:

> Please use [PATCH v2] for version 2 of a patch, etc. git format-patch
> --subject-prefix "PATCH v2"

thanks, I'll do it.

> This seems overkill. Why can't we add an argument to show() which skips the
> reset_state step?

>
> Hmm, I guess someone could reopen the connect wizard while the first
> connection is opening, which would mean the state is wrong when we re-show the
> wizard.
>
> Thinking some more, what we really want here is:
>
> Launch 'Open Connection', set parameters, hit 'connect'
> 'Open Connection' window is desensitized, async progress meter dialog opens up
> while connection is connecting
> If connection fails, we drop back to the still open connection wizard with
> same state still listed.
>
> That's how the create/clone/delete wizards work, basically make the operation
> synchronous. However truth be told I don't care enough about this bug to
> mandate that we do that. If you want to look into it it might be simple, or it
> might be a pain.
>
> But the above solution is definitely not ideal since it requires duplicating
> update of UI elements in two places (set_state and reset_state) with the
> latter in a rarely tested code path.

I have firstly implemented it because I wanted to take care of that
case but I agree it is a very uncommon case.  Then I realized that
set_state can also be useful if we intent to support "Modify connection"
in future (not really sure it is useful).

Would you accept the patch if I combine reset_state in set_state, so
that set_state will accept a set_to_default argument?  If you still
think that the cost of maintaining the new code is bigger than the
advantage of fixing that particular case, I can drop set_state.

Thanks,
Giuseppe


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